-
Notifications
You must be signed in to change notification settings - Fork 20
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
Rewrite normalize action #441
Conversation
91f68ea
to
07f612d
Compare
Signed-off-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
07f612d
to
7e56968
Compare
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.
Overall LGTM. The one thing I don't love is the API break (which this PR description should mention). I'd rather define and immediately deprecate pipe operators to keep the no argument variant operational instead of dropping.
Signed-off-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
d5b93c5
to
185f744
Compare
Fixed in 185f744. |
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.
Awesome! All the actions should take this structure, right? If so, I can help with the overlay action.
@DPR00 Yeah, that would be great! |
### Proposed changes Rewrite overlay function following the guidelines in #441. #### Type of change - [ ] 🐛 Bugfix (change which fixes an issue) - [x] 🚀 Feature (change which adds functionality) - [ ] 📚 Documentation (change which fixes or extends documentation) ### Checklist - [x] Lint and unit tests (if any) pass locally with my changes - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have added necessary documentation (if appropriate) - [x] All commits have been signed for [DCO](https://developercertificate.org/) --------- Signed-off-by: Diego Palma <dpalma@symbotic.com> Co-authored-by: Diego Palma <dpalma@symbotic.com>
Proposed changes
Trying a new way of implementing actions that gets rid of the argument re-ordering madness and is hopefully easier to maintain.
Type of change
Checklist