Allow new syntax for enum to avoid leading _ from reserved options#41328
Merged
kamipo merged 1 commit intorails:mainfrom Feb 19, 2021
Merged
Allow new syntax for enum to avoid leading _ from reserved options#41328kamipo merged 1 commit intorails:mainfrom
enum to avoid leading _ from reserved options#41328kamipo merged 1 commit intorails:mainfrom
Conversation
kamipo
force-pushed
the
avoid-leading-_-from-reserved-options
branch
from
f46743a to
02ad7ba
Compare
kaspth
approved these changes
Feb 9, 2021
rafaelfranca
reviewed
Feb 9, 2021
Member
There was a problem hiding this comment.
Should we deprecate all those options and the old syntax? We can probably include a Cop to do the migration automatically.
Member
Author
There was a problem hiding this comment.
It is a good idea to me if happening the migration is ok for us.
I thought since it is confusing that the old syntax options has leading _ but the new syntax options doesn't have that, I'd also like to deprecate defining enum on the attribute names (:prefix, :suffix, :scopes, and :default) in the old syntax to reserve the names to make it as options in the future.
Member
There was a problem hiding this comment.
We don't need to deprecate in this release, but if we can easily autocorrect the deprecation with a cop, I think we should.
rafaelfranca
approved these changes
Feb 10, 2021
kaspth
approved these changes
Feb 15, 2021
Contributor
kaspth
left a comment
kaspth
left a comment
There was a problem hiding this comment.
One more question, but ultimately 👍
Unlike other features built on Attribute API, reserved options for `enum` has leading `_`. * `_prefix`/`_suffix`: rails#19813, rails#20999 * `_scopes`: rails#34605 * `_default`: rails#39820 That is due to `enum` takes one hash argument only, which contains both enum definitions and reserved options. I propose new syntax for `enum` to avoid leading `_` from reserved options, by allowing `enum(attr_name, ..., **options)` more Attribute API like syntax. Before: ```ruby class Book < ActiveRecord::Base enum status: [ :proposed, :written ], _prefix: true, _scopes: false enum cover: [ :hard, :soft ], _suffix: true, _default: :hard end ``` After: ```ruby class Book < ActiveRecord::Base enum :status, [ :proposed, :written ], prefix: true, scopes: false enum :cover, [ :hard, :soft ], suffix: true, default: :hard end ```
kamipo
force-pushed
the
avoid-leading-_-from-reserved-options
branch
from
3e8bf3c to
0618d2d
Compare
4 tasks
4 tasks
hendrixfan
added a commit
to hendrixfan/mastodon
that referenced
this pull request
Feb 15, 2024
As of Rails 7.0, the enum syntax has been updated [1] to positional arguments without the prefix `_`. The old syntax is still supported [2], but it is recommended to use the new one. [1] rails/rails#41328 [2] rails/rails#50987 `
hendrixfan
added a commit
to hendrixfan/mastodon
that referenced
this pull request
Feb 15, 2024
As of Rails 7.0, the enum syntax has been updated [1] to positional arguments without the prefix `_`. The old syntax is still supported [2], but it is recommended to use the new one. [1] rails/rails#41328 [2] rails/rails#50987 `
hendrixfan
added a commit
to hendrixfan/mastodon
that referenced
this pull request
Feb 15, 2024
As of Rails 7.0, the enum syntax has been updated [1] to positional arguments without the prefix `_`. The old syntax is still supported [2], but it is recommended to use the new one. [1] rails/rails#41328 [2] rails/rails#50987 `
9 tasks
9 tasks
tobyprivett
added a commit
to ministryofjustice/hmpps-book-secure-move-api
that referenced
this pull request
Oct 10, 2024
kamipo
added a commit
to kamipo/rails
that referenced
this pull request
Oct 22, 2024
Enum accepts labels as keyword arguments is originally intended in rails#41328, so I documented and covered by test that. https://github.com/rails/rails/blob/0618d2d84a501aea93c898aec504ff9a0e09d6f2/activerecord/lib/active_record/enum.rb#L60-L65 https://github.com/rails/rails/blob/0618d2d84a501aea93c898aec504ff9a0e09d6f2/activerecord/test/cases/enum_test.rb#L702-L713 I'm not against dropping that support to simplify codebase, at least we should deprecation that before removing. Fixes rails#53407.
tobyprivett
added a commit
to ministryofjustice/hmpps-book-secure-move-api
that referenced
this pull request
Nov 11, 2024
tobyprivett
added a commit
to ministryofjustice/hmpps-book-secure-move-api
that referenced
this pull request
Nov 15, 2024
tobyprivett
added a commit
to ministryofjustice/hmpps-book-secure-move-api
that referenced
this pull request
Nov 18, 2024
* Rails 8 beta * Migrate to new enum rails/rails#41328 * Resolve: DEPRECATION WARNING: `to_time` will always preserve the full timezone * Update spec with new error message * Rails 8 prefers flipper-active_record from HEAD * Update Rails to 8.0.0 * Run `rails app:update` * Load Rails 8.0 defaults * Run `bundle update` * Remove `to_time_preserves_timezone` as it is a Rails 8 default * Remove flipper - The gem is outdated and the feature flag was last used once, 5 years ago * Pin rubocop to a commit to avoid continual dependabot bumps - Hopefully GDS will tag a new release and we won't have to do this anymore
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unlike other features built on Attribute API, reserved options for
enumhas leading_._prefix/_suffix: Add prefix option to enum definition #19813, Rename the enum_prefix and enum_suffix options to _prefix and _suffix #20999_scopes: option to disable all scopes thatActiveRecord.enumgenerates #34605_default: Allow default to be configured for Enum #39820That is due to
enumtakes one hash argument only, which contains bothenum definitions and reserved options.
I propose new syntax for
enumto avoid leading_from reservedoptions, by allowing
enum(attr_name, ..., **options)more AttributeAPI like syntax.
Before:
After: