Skip to content

Allow invocations of indexes to accept block arguments. #257

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

Closed

Conversation

TreyE
Copy link

@TreyE TreyE commented May 20, 2024

Indexing of elements in Ruby is really just an invocation of the [] method on an object. As such, element indexing should accept a block.

The pull request contains tests as well as an extension of the grammar.

@TreyE
Copy link
Author

TreyE commented Jun 30, 2024

Wow, so we're just gonna grab my code and submit it as your own under PR #262 @hvitved ? Not even a mention on the original PR?

Very uncool.

I don't care about the credit, but a thanks would have been nice. At the very least you could have closed the PR so it doesn't stay open instead of ignoring it entirely.

@amaanq is this the normal submission process? Because if contributions are going to just be taken without any kind of attribution I'll just stop now - and I'll let my colleagues know to do the same.

@hvitved
Copy link
Contributor

hvitved commented Jul 1, 2024

Sorry, I had forgotten to look at already open PRs before submitting my fix (which was needed for github/codeql#16757).

@amaanq
Copy link
Member

amaanq commented Jul 5, 2024

Hey @TreyE, I am really sorry for not noticing the open PR as well - the Ruby grammar is one I do not check in on as much as the others, so I hadn't seen you PR in the same fix here, and I saw hvitved's because he pinged me.

Going forward I'll be sure to double check all open PRs to avoid that happening again, really sorry. I would still appreciate any and all fixes/improvements you want to make (if you are still interested, understandable if you are not), and feel totally free to just ping me to take a look 🙂

@amaanq amaanq closed this Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants