Skip to content

Conversation

ghiculescu
Copy link
Member

@ghiculescu ghiculescu commented Jan 16, 2023

Attributes such as set and reset should not be used as they clash with the CurrentAttributes public API. This PR raises an ArgumentError if a restricted attribute name is used.

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.

…ame is used.

Attributes such as `set` and `reset` should not be used as they clash with the  `CurrentAttributes` public API. This PR raises an `ArgumentError` if a restricted attribute name is used.
@ghiculescu ghiculescu force-pushed the current-attributes-restricted-names branch from 6b92328 to 1284df5 Compare January 16, 2023 00:07
@ghiculescu
Copy link
Member Author

This might be a breaking change (but anyone relying on it would have not had current attributes work properly), so maybe a deprecation is needed?

@byroot byroot merged commit f1b1ad6 into rails:main Jan 16, 2023
@byroot
Copy link
Member

byroot commented Jan 16, 2023

I don't think a warning is needed, as like you point out, it would have been broken in various ways.

@ghiculescu ghiculescu deleted the current-attributes-restricted-names branch January 16, 2023 15:13
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.

2 participants