Skip to content

Comments

Support validation for old Enum API#50333

Closed
skipkayhil wants to merge 1 commit intorails:mainfrom
skipkayhil:hm-validate-old-enum
Closed

Support validation for old Enum API#50333
skipkayhil wants to merge 1 commit intorails:mainfrom
skipkayhil:hm-validate-old-enum

Conversation

@skipkayhil
Copy link
Member

Motivation / Background

The new, symbol, #enum API recently gained support for a new validate option which replaces ArgumentErrors with Active Model validation errors.

Detail

This commit adds _validate to the old, keyword argument, #enum API to keep the two APIs in sync.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

The new, symbol, `#enum` API recently [gained][1] support for a new
`validate` option which replaces `ArgumentErrors` with Active Model
validation errors.

This commit adds `_validate` to the old, keyword argument, `#enum` API
to keep the two APIs in sync.

[1]: 7c65a4b
@p8
Copy link
Member

p8 commented Dec 17, 2023

This makes sense to me to keep API's consistent.
Are these underscored options going to be deprecated eventually?

@skipkayhil
Copy link
Member Author

Are these underscored options going to be deprecated eventually?

I think deprecating makes sense, the underscored options are definitely less user friendly and it seems really easy for the two sets of options to go out of sync. (In that case, we probably don't need this PR as the recommendation would be to just migrate to the new syntax)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants