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

💅 use ENV.fetch(name, default) form #47307

Merged
merged 1 commit into from Feb 8, 2023
Merged

Conversation

zzak
Copy link
Member

@zzak zzak commented Feb 8, 2023

Follow up to #47143, prefers the ENV.fetch(name, default) form instead of block form.

Block form is only useful if we want to yield the name, I think.

[ref]

@rails-bot rails-bot bot added the railties label Feb 8, 2023
@natematykiewicz
Copy link
Contributor

Block form is also usually used when the default value is expensive to compute, since it only needs to be computed when the key doesn't exist. For static values, it's less code and faster to use 2 arguments and no block.

There's a Rubocop rule to prefer 2 arguments when the default is a simple static value.
rubocop/rubocop#8147

@guilleiguaran guilleiguaran merged commit ad83b01 into rails:main Feb 8, 2023
@zzak zzak deleted the re-47143 branch February 8, 2023 23:48
@zzak
Copy link
Member Author

zzak commented Feb 9, 2023

Good call @natematykiewicz, and thank you for explaining the differences to me.

I've opened #47327 to autocorrect any violations for that rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants