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

Allow for fixup! {conventional} #392

Open
JP-Ellis opened this issue Sep 3, 2024 · 8 comments · May be fixed by #397
Open

Allow for fixup! {conventional} #392

JP-Ellis opened this issue Sep 3, 2024 · 8 comments · May be fixed by #397
Labels
enhancement Improve the expected question Uncertainty is involved

Comments

@JP-Ellis
Copy link

JP-Ellis commented Sep 3, 2024

It would appear to me that the following combination of configuration options

style = "conventional"
no_fixup = false

Should allow for fixup to conventional commits. Unfortunately, this is not what is observed:

echo 'fixup! chore: testing' | committed   
-: error Commit is not in Conventional format: Missing type in the commit summary, expected `type: description`

My expectation is that a fixup to a conventional commit should be allowed, and that committed should verify the message to be of the form fixup! type: description.

@epage epage added enhancement Improve the expected question Uncertainty is involved labels Sep 3, 2024
@epage
Copy link
Collaborator

epage commented Sep 3, 2024

That is an interesting one because, technically, it isn't a Conventional Commit anymore but that only matters if this isn't a transient state.

@JP-Ellis
Copy link
Author

JP-Ellis commented Sep 3, 2024

Completely agree, it is just a transient state, but it is a common workflow.

For example, I might be working on a PR with a few logically distinct commits, and I realise that one of the earlier commits has an error. I will create a fixup! commit temporarily, and then before rebasing the PR, I'll perform a rebase to automatically remove these fixes.

While implementing this, it should be noted that the same logic should apply for squash! commits, though the latter have the additional complexity of allowing the commit message to be amended.

Depending on how deep you want to go with this implementation, the ideal scenario would be to have the option to allow/forbid fixup! and squash! commits on a per branch basis. Specifically, forbidding fixup! and squash! on main and other long-lived branches, and then allowing these commits on all other branches.

@JP-Ellis
Copy link
Author

JP-Ellis commented Sep 3, 2024

If this is something you would be happy to see added to committed, I would be happy to work on the feature.

I might not have much time in the next week or so, but I could start end of next week or so.

@epage
Copy link
Collaborator

epage commented Sep 4, 2024

For example, I might be working on a PR with a few logically distinct commits, and I realise that one of the earlier commits has an error. I will create a fixup! commit temporarily, and then before rebasing the PR, I'll perform a rebase to automatically remove these fixes.

To clarify, are you pushing your fixups or only keeping them local?

I've been considering some way to distinguish between local runs and CI runs. For example, I regularly have a WIP commit but I'd want CI to fail if thats present.

@epage
Copy link
Collaborator

epage commented Sep 16, 2024

#394 jumps the gun. I did not say we were good for going forward. I was looking for a better understanding of the overall workflow to better understand what should happen.

@JP-Ellis
Copy link
Author

No worries!

@JP-Ellis JP-Ellis closed this as not planned Won't fix, can't repro, duplicate, stale Sep 16, 2024
@JP-Ellis
Copy link
Author

I've been considering some way to distinguish between local runs and CI runs. For example, I regularly have a WIP commit but I'd want CI to fail if thats present.

Within a PR, such commits might very well exist; but they should not land in the long lived branch.

This could be either because multiple people are working on the same PR, and therefore performing a rebase would be too disruptive. Alternatively, I might intend to squash-and-rebase the PR anyway, in which case the commits in the PR are effectively irrelevant.

I'll leave you reopen this if/when you want this implemented. If you have no intentions of doing so, let me know and I'll create a fork for my use cases.

@epage epage reopened this Sep 19, 2024
@epage epage linked a pull request Sep 19, 2024 that will close this issue
@epage
Copy link
Collaborator

epage commented Sep 19, 2024

I'll leave you reopen this if/when you want this implemented. If you have no intentions of doing so, let me know and I'll create a fork for my use cases.

Just because its undecided, doesn't mean it should be closed. For me, closing is for its rejected outright. I do not generally explore rejected issues for re-evaluating opening them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve the expected question Uncertainty is involved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants