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

Unite Active Storage configs in load_defaults '6.1' into one if-section #41054

Conversation

bogdanvlviv
Copy link
Contributor

When I was comparing 'defaults' for 6.1 in this method and our configuring
guide, I was confused that some active_storage options are missing.

This change doesn't bring any implementation changes and feels like
a cosmetic change. Please feel free to close this if you think so and don't
see that we could benefit this change.

@rails-bot rails-bot bot added the railties label Jan 8, 2021
@kaspth
Copy link
Contributor

kaspth commented Jan 8, 2021

I see your point but now the two queues resetting sections are away from each other. What about moving the variants config to the queues section? (and adding a newline in between them).

…tion

When I was comparing 'defaults' for 6.1 in this method and our configuring
guide, I was confused that some active_storage options are missing.

This change doesn't bring any implementation changes and feels like
a cosmetic change. Please feel free to close this if you think so and don't
see that we could benefit this change.
@bogdanvlviv bogdanvlviv force-pushed the unify-active_storage-configs-in_load_defaults_6_1 branch from c6c8f51 to 5e0d451 Compare January 8, 2021 13:31
@bogdanvlviv
Copy link
Contributor Author

@kaspth It is a good idea. I've amended this PR.

@kaspth kaspth merged commit 555aa38 into rails:master Jan 8, 2021
@kaspth
Copy link
Contributor

kaspth commented Jan 8, 2021

Awesome! Exactly this, thank you 🙌

@bogdanvlviv bogdanvlviv deleted the unify-active_storage-configs-in_load_defaults_6_1 branch January 8, 2021 13:52
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

2 participants