-
Notifications
You must be signed in to change notification settings - Fork 337
Conversation
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.
Super excited to see this!!!! I have some comments below :D
0e4dfe0
to
b089cf7
Compare
b089cf7
to
bc74f97
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.
I still think there's more we can simplify here :) I can take a crack at a PoC and see what you think. I also think it would be SUPER to have a little ascii comment with the truth table fully drawn out--this would greatly help other maintainers understand this complex little bit of code.
I've been talking with the Ashley's about this PR and the general feeling is that this is an improvement to what is there already, and this PR is intended to deprecate release and not to entirely rework how we do this. I'm open to getting a refactor in (I think the way to do this is to split stuff out into a deploy config struct of some sort) which will improve readability and maintainability but for now I think I should probably just add some more comments. (yay for small incremental improvements) :) @gabbifish if you feel like taking on a big refactor feel free and let me know if you have questions - we have some time before 1.5.0 so I'm going to step back from this PR and work on some other stuff |
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.
one small ask left :D
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.
the truth table comment is AWESOME. Thanks for writing it up!!!
Breaking Changes!
Fixes #538
This PR removes the ability to pass
--release
towrangler publish
.If we plan on doing a point release before 1.5.0 then this should wait.