-
Notifications
You must be signed in to change notification settings - Fork 131
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
Make the filter name configurable independently of the attributes #204
Conversation
That makes it pretty much a no-no. No breaking changes allowed anymore. Is there any way to make this non-breaking? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, in the interest of preventing regressions, you should never edit existing unit tests. Only add new ones. This ensures that existing code keeps working.
Oh, I didn't expect you were still reviewing the PRs here. I created it more for the record. Sorry. If there's a chance that changes still get integrated, I would put more work into the PR to make it a non-breaking change. |
If it's purely additive and non-breaking, sure, why not. |
I have updated the PR to make this a non-breaking change as requested. If you don't specify the new Also, I have added new tests instead of changing the existing ones. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, thanks a lot!
Released to NPM as |
Thank you for the very quick release! |
We had a case where we needed to use a custom filter, but continue to use the regular
angular-gettext
directive. So we need to configure the filter name independently of the attributes. This PR does that.Projects using a custom attribute and filter will need to set the
filterName
option accordingly, so this is a breaking changes.