Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates #294

Merged
merged 3 commits into from
May 6, 2024
Merged

Updates #294

merged 3 commits into from
May 6, 2024

Conversation

amaanq
Copy link
Member

@amaanq amaanq commented May 6, 2024

(Maybe) Closes #244
Closes #271
Closes #287
Closes #289
Closes #292
Closes #293

@amaanq amaanq force-pushed the updates branch 2 times, most recently from 47f27bf to 30cf076 Compare May 6, 2024 04:51
@amaanq amaanq merged commit c315722 into master May 6, 2024
5 checks passed
@amaanq amaanq mentioned this pull request May 6, 2024
5 tasks
@dcreager
Copy link
Contributor

dcreager commented May 6, 2024

This PR was opened, and then merged 20 minutes later with no review. It looks like it was largely about updating the language bindings, and so I realize there wasn't a lot of "meat" to review. But I still think it's important to get another set of 👀 on every PR, even the superficial ones. (Especially the large superficial ones!) In this case, you could have provided a PR description calling out that it's largely boilerplate changes, and not substantially touching the actual grammar, which would have made the reviewer's job much easier.

@amaanq
Copy link
Member Author

amaanq commented May 6, 2024

I guess, I just made the same changes in a bunch of upstream repos (php, regex, ruby, embedded template, etc) so I don't think it really warranted review since they were, by and large, the exact same changes already incorporated in other grammars (rust, js, go, etc)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment