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

Rename parameter_overwite_by_rails_rule to parameter_overwrite_by_rails_rule #396

Merged
merged 3 commits into from
May 16, 2024

Conversation

r7kamura
Copy link
Contributor

@r7kamura r7kamura commented Jul 4, 2023

"overwite" is probably a typo for "overwrite" ?

Copy link
Member

@ydah ydah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option has already been released in v5.0.0, so it would be a good to change it while maintaining backwards compatibility.
It would be a good to proceed as follows in stages.

  1. Add parameter_overwrite_by_rails_rule option to indicate to use it (or print a warning)
  2. Release the above as a minor version update
  3. Delete parameter_ overwrite _by_rails_rule during the next major version update

@brandur
Copy link
Member

brandur commented May 13, 2024

Excellent point @ydah! It'd be great to fix this, and we should, but given that it was released already, we should be a little more careful about it.

@r7kamura Thank you for change! Sorry about the long delay here, but if you make the requested update above, we'll try to get it ushered in.

@r7kamura
Copy link
Contributor Author

OK, I've added one more commit to support the "overwite" one as well, so please check it out.

Copy link
Member

@ydah ydah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@r7kamura Thank you! Looks good!
I think this is a change that should be communicated to users, so could you add a description of the change in the CHANGELOG?

Copy link
Member

@ydah ydah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@ydah ydah merged commit ede3552 into interagent:master May 16, 2024
6 checks passed
@r7kamura r7kamura deleted the overwite branch May 16, 2024 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants