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

fix: pass block argument in ActiveRecord find_by_sql patch #1259

Merged

Conversation

olepbr
Copy link
Contributor

@olepbr olepbr commented Nov 20, 2024

find_by_sql takes a block as its final argument, which is neither a positional nor a keyword argument, hence we need to pass it explicitly; before this PR, the block was not passed to the super() call and hence not executed, thus altering application behavior.

Fixes #1258.

Copy link

linux-foundation-easycla bot commented Nov 20, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@arielvalentin
Copy link
Collaborator

@olepbr in addition to my comments, you are required to sign the EasyCLA before we are able to review and accept your contribution.

Thanks.

`find_by_sql` takes a block as its final argument, which is neither a
positional nor a keyword argument, hence we need to pass it explicitly;
before this commit, the block was not passed to the `super()` call and
hence not executed, thus altering application behavior.

Fixes open-telemetry#1258
@olepbr olepbr force-pushed the pass-block-in-activerecord-patch branch from 4f4bb18 to e17cf0f Compare November 20, 2024 23:02
@olepbr olepbr force-pushed the pass-block-in-activerecord-patch branch from a2331b9 to 99a676d Compare November 21, 2024 09:10
@olepbr
Copy link
Contributor Author

olepbr commented Nov 21, 2024

CI was failing due to a Rubocop offense triggered by the added tests; amended now.

@arielvalentin arielvalentin merged commit 16bef85 into open-telemetry:main Nov 21, 2024
57 checks passed
@github-actions github-actions bot mentioned this pull request Nov 21, 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.

New ActiveRecord find_by_sql patch fails to pass &block argument
3 participants