feat!: rename fixup to autosquash #396
Conversation
| @@ -63,7 +63,7 @@ pub(crate) enum Content<'s> { | |||
| NoPunctuation(NoPunctuation), | |||
| Imperative(Imperative<'s>), | |||
| Wip(Wip), | |||
| Fixup(Fixup), | |||
| Autosquash(Autosquash), | |||
There was a problem hiding this comment.
When splitting out a PR, please address feedback from the old one as its easy for it to drop through the cracks otherwise, see #394 (comment)
The PR is marked as a breaking change with ! but it doesn't say how its a breaking change and I don't think making a breaking change for this is worth it.
If you still want to move forward with this, you could leave the message as fixup and but have an alias for autosquash and an issue tracking the switch over / removing of aliases.
There was a problem hiding this comment.
The PR is marked as a breaking change with ! but it doesn't say how its a breaking change
I believed that the title of the PR and commit was self explanatory. If there's anything unclear about it, please consider adjusting the commit message when or if you squash and rebase.
There was a problem hiding this comment.
I am asking because it is not clear. Even if it was, I would ask that you not out source fixing of your PRs to maintainers as that is not a scalable solution as there are significantly more contributors to maintainers.
There was a problem hiding this comment.
I have amended the commit message to make the breaking change clear; however, I am still unsure whether this is something you would like or reject, or would require amendments to.
You suggested inverting the logic to at an autosquash alias. Is this a requirement to get this merged? Personally, keeping the name fixup when it handles all of Git's autosquash commits would be confusing to maintain later on; but I understand that from a functional perspective, the alias works symmetrically.
There was a problem hiding this comment.
I have amended the commit message to make the breaking change clear;
The commit (but not PR) says
BREAKING CHANGE: rename fixup to autosquash
That doesn't say in what contexts. In most cases, there is no breaking change for this iiuc. this is specifically scoped to the json messages that are emitted.
You suggested inverting the logic to at an autosquash alias. Is this a requirement to get this merged?
My suggestion is specifically for the json messages we emit. By removing that breaking change (and ideally having no other), it changes this PR from a "unsure" to "yeah, lets merge this".
As I said, we can track re-invert of the json message in an issue along with removing the references to fixup. This might not be there always but it will likely be there for a while.
Pull Request Test Coverage Report for Build 10936294054Details
💛 - Coveralls |
The `fixup!` commit is just one instance of the more general autosquash feature as described in [Git's `--autosquash` docs](https://git-scm.com/docs/git-rebase#Documentation/git-rebase.txt---autosquash) This renames instances of 'fixup' to 'autosquash', while creating aliases to provide a smoother transition to end users. BREAKING CHANGE: rename fixup to autosquash Signed-off-by: JP-Ellis <[email protected]>
JP-Ellis
force-pushed
the
feat/fixup-config-rename
branch
from
5919652 to
c0cda15
Compare
The
fixup!commit is just one instance of the more general autosquash feature as described in Git's--autosquashdocs
This renames instances of 'fixup' to 'autosquash', while creating aliases to provide a smoother transition to end users.
See the related PR in #395 for the motivation for this change.