Squash-merges about commit-analyzer HOT 24 CLOSED

I would agree that it is a pain point not having semantic-release trigger a version bump when you squash-merge.

from commit-analyzer.

echoing @nfantone

I forked commit-analyzer (latest) and added #66, works great!

Is there a reason not to do this?

from commit-analyzer.

Thw motivation here is, I tend to do my merges from GitHub, and since GitHub remembers my previous merge type, it's easy to accidentally squash something. If that happens, and I don't edit the squash commit message, semantic-release is going to do the "wrong thing" (although only in the case where there's a "feat" no breaking change involved). I'm not the only person in the world who merges from GitHub, so I'm sure I'm not the only person who is going to run into this problem.

from commit-analyzer.

I would also point out that a big part of what "brought me around" is understanding how conventional-changelog works. But, making decisions on how a product should behave based on it's inner workings is probably the exact opposite of good UX. The average user of semantic-release doesn't care about conventional-changelog, or the inner workings of semantic-release.

At the end of the day, if I squash-merge a commit with a breaking-change, semantic-release picks up the breaking change. If I squash-merge a commit with a "feat", semantic-release doesn't pick up the "feat". Semantic-release is not consistent in how it behaves, which is weird.

from commit-analyzer.

I haven't. I was thinking the best way to implement such a thing would be to write essentially a wrapper for commit-analyzer which would parse out the squashed commit into individual commits and feed them into commit-analyzer (or into whatever plugin you want to use in place of commit-analyzer). By all means, have at it. :)

from commit-analyzer.

We use squash merge into master as well and this has resulted in a lot confusion around when semantic-release does trigger a release and when it doesn't.

In squash merge on GitHub, only PR title is used to calculate the release status so it would be great to have a community plugin which extends the commit-analyzer behaviour.

I do understand @pvdlg 's concerns about the "correctness" of the proposed solution. However, I am in the same opinion as @jwalton that parsing squash messages would improve the UX for many users. Updating the squash message title so semantic-release is happy is suboptimal. We should not become the tool of our tools.

@jwalton have you released PR #66 as a separate package? If not, I would like to have a go at turning that into a package.

from commit-analyzer.

Is there an update to this?
Is the proposed solution to change the squash message and remove the first line and the stars?

Title of PR (#91)

* style(UI-115): reformat

* fix(UI-115): remove table border

would need to be changed to:

style(UI-115): reformat


fix(UI-115): remove table border

from commit-analyzer.

Had the same situation, The tl;dr version is:

• For new features / fixes pull requests use squash commits and make sure the title of the pull request is a conventional commit one. (Each PR is one entry in the changelog), you can even use a tool like https://github.com/zeke/semantic-pull-requests to lint PR titles.
• When merging to release branch, remember to not use squash commit strategy so that it includes all the other squashed PRs separately in the changelog instead of the merge commit which probably won't trigger a release.

from commit-analyzer.

@jwalton @pvdlg We have a very real need for this at work. May I ask why was this never implemented and/or closed? I can take over the work if there's additional things to be done.

Is this still the position we're adopting?

I was thinking the best way to implement such a thing would be to write essentially a wrapper for commit-analyzer which would parse out the squashed commit into individual commits and feed them into commit-analyzer.

That comment was from 2 years ago. Was such a package ever released?

from commit-analyzer.

semantic-release is based on established commit conventions (Angular, ESLint etc...).
All those convention define that a commit message must have a certain subject and body and purposefully ignore the case of squashed commits.

In addition, implementing what you propose would prevent to use squashing to change commit message. It happens very often we get PRs with a lot of different commits that contains "wrong" messages, so we use squashing to fix the message.
For example if someone opens a PR that fix something but write the commit message as feat: fix something we can "Squash and Merge" on GitHub to change the message to fix: fix something.
If we were to implement what you suggest semantic-release would use the original feat message rather than the new one we actually want.

Also I'm not sure that Squashed commit of the following: is a norm. So I don't know if parsing that would be reliable. I imagine different tools (and Git host) use different keywords to flag a squashed commit, and semantic-release would have to work with all of them.

So I would be mostly against implementing such feature in our default plugin. However if you really want this feature I would suggest to create your own plugin.

from commit-analyzer.

In addition, implementing what you propose would prevent to use squashing to change commit message

Not at all; I have this going in a fork right now, and my current implementatiom will only try to parse the squash merge if the first line is "Squashed commit of the following:", and if the rest of the message can be correctly parsed. It has to be one-or-more 'commit header, author header, date header, followed by indented commit messages". If it doesn't conform to that in any way, it falls back to treating it like one big commit message.

If you change the top header, then it works exactly the same way it does today. My proposed change only takes effect when it's a straight squash merge.

(Although, I could take this a step further. Today if you squash merge a big commit, and change the top line to fix things, and you don't notice that somewhere in the many commits there's a "BREAKING CHANGE", the current commit analyzer will take the breaking change and get you a major release - we could see if the "tail" of the message is a squash merge - if it is, and the "head" is modified, then only use the head and purposefully ignore the tail, but that's perhaps getting too fancy).

But for the straight-up squash merge, where you don't edit the commit message, it's a collection of commits all merged into one. You have all the original commit messages right there, which follow conventional commit. Why would you purposefully ignore them? If there's a "feat: ..." In there somewhere, you should be doing a minor instead of a patch no release.

from commit-analyzer.

Thw motivation here is, I tend to do my merges from GitHub, and since GitHub remembers my previous merge type, it's easy to accidentally squash something.

I find it inconvenient as well that Github selects by default the last type of merge. I think it's quite error prone. However it's a GitHub issue not a semantic-release one. I think Github should improve that, rather than modifying semantic-release to work around it.

semantic-release aims at being more than just an automated way to make releases. The idea is to enforce good practices, such as having a clear commit history and make atomic commits.
We recommend to:

• make 1 commit per fix or feature
• make sure each commit make sense on it's own
• make sure the builds and tests passes for each commit.

That makes it a lot easier to see the history of changes in the codebase as you can look at the list of commits subject and immediately know what is the purpose of each one.

Therefore squashing commits and using the body of the squashed commit to explain what is the change in the codebase is not a really good practice. When you list your commits you cannot see immediately which commit changes what. You would have to read each commit body to figure that out.

Certain tools might parse the squashed commit message body to display them in the UI as if they were multiple commits, but it's not the case for many Git tools (including the git CLI and GitHub itself).

In addition doing this type of squashing will make it a lot harder to use git blame as a given change in the codebase will be associated to a squashed commit that contains many unrelated things (i.e. the modifications done in the other commits before squashing).

For all this reasons I don't think we should include this change in the default plugin as it would encourage some "bad practices" (I put that in quotes as it's not inherently bad but creates certain inconveniences mentioned above). The plugin system allows you to implement whatever parsing you want, so I think this feature should be implemented in a "community plugin" instead.

semantic-release is meant to be agnostic regarding the outside tooling (Git host, Git IDE etc...) used by the project. It seems the desire for this feature comes from a specific behavior of GitHub happening in a very specific use case and relies on the specific GitHub formatting of squashed commit (Squashed commit of the following:) so that's another reason why it should be implemented in a "community plugin" rather than in this one.

We use conventional-changelog to do the parsing and the changelog creation. It's currently the closest thing to a commit message specification we have and they don't parse squashed commit message body. So I would rather stick to whatever conventional-changelog implements as they kind of define what the standard is. In addition, sticking to conventional-changelog allows to use multiple tools that relies on commit message (so if your commit messages works with semantic-release they will work with any other tools based on conventional-changelog).

On a more technical standpoint, if you would implement what you propose it's going to create an issue with the changelog generation (which is also based on conventional-changelog) as the entire system has been designed around the idea of 1 commit = 1 atomic change = 1 line in the changelog. With what you did in #66, the final changelog generated will not be what you expect as the the squashed commit will not be included, nor the "original" commits.

from commit-analyzer.

relies on the specific GitHub formatting of squashed commit

Small correction; this formatting is not from GitHub, it's from git. Try doing git merge --squash mybranch && git commit from the command line, and you'll get exactly the same message. Semantic-release may be tool agnostic for the most part, but it's definitely not git agnostic. 🙂

With what you did in #66, the final changelog generated will not be what you expect as the the squashed commit will not be included, nor the "original" commits.

Actually, the changelog plugin handles this pretty nicely. It generates:

# 2.0.0 (https://github.com/exegesis-js/exegesis-plugin-roles/compare/v1.0.1...v2.0.0) (2018-05-18)

    * Squashed commit of the following: (1817566 (https://github.com/exegesis-js/exegesis-plugin-roles/commit/1817566)), closes #20 (https://github.com/exegesis-js/exegesis-plugin-roles/issues/20)

### BREAKING CHANGES

    * This is a breaking change.

It might not list things under new features the way you'd expect, but it does an OK job, and at least we don't release a "feat" as a "fix", or a "fix" as a "breaking change" by accident. It's easy to go fix the release notes if semantic-release gets them wrong. It's considerably more complicated to undo a release.

from commit-analyzer.

The changlog plugin recognizes it in that specific case because it’s parsed as a breaking change. Without the BREAKING CHANGE it would be ignored.

For all the reasons mentioned above I still it’s better suited for a community plugin. You can also propose the change to conventional-changelog and see what they say. That would at least solve the consistency issue between the commit analyzer and the changelog generator plugin.

from commit-analyzer.

You're slowly bringing me around, actually.

The one thing is, conventional-changelog has explicit support for squashed commits. It does treat them specially.

If I squash merge a bunch of commits, and then I merge a bugfix, I'll get a changelog that looks like:

<a name="0.0.0-semantic-release"></a>
# [0.0.0-semantic-release](https://github.com/exegesis-js/exegesis-plugin-roles/compare/v1.0.1...v0.0.0-semantic-release) (2018-05-18)


* Squashed commit of the following: ([08f2a44](https://github.com/exegesis-js/exegesis-plugin-roles/commit/08f2a44)), closes [#20](https://github.com/exegesis-js/exegesis-plugin-roles/issues/20)


### Bug Fixes

* Fix ([a74e64c](https://github.com/exegesis-js/exegesis-plugin-roles/commit/a74e64c))


### BREAKING CHANGES

* This is a breaking change.

So conventional-changelog didn't ignore the commit, like it would for a random commit that doesn't match the angular commit pattern. But it also didn't categorize it (aside from the BREAKING CHANGE part). semantic-release, on the other hand, just ignores it, unless it's breaking.

Maybe this just needs a really good entry in the FAQ...

from commit-analyzer.

The average user of semantic-release doesn't care about conventional-changelog, or the inner workings of semantic-release.

In that case conventional-changelog is not the inner working of semantic-release. It's the commit specification the user has to follow. So the user is very much concerned about that.
This is actually why the first paragraph of the semantic-release documentation is about the commit convention. It's the most important part and every users/contributors have to read and follow the commit convention.

In the case of squashed commits, by default GitHub creates a commit message that doesn't follow the commit convention, therefore it's ignored by semantic-release.
The entire workflow relies on following the convention, if you don't follows it, nothing works as expected, whether if the invalid commit message come from a typo or a default Github message.

from commit-analyzer.

There is tools like commitlint to enforce valid commit messages.
In any case, it's not in the scope of semantic-release to lint the commit messages for now.
And it will probably never be in the scope of semantic-release to workaround messages that doesn't comply with the convention. If the convention evolves then semantic-release will follow the new convention.

from commit-analyzer.

So I would rather stick to whatever conventional-changelog implements as they kind of define what the standard is.

In the case of squashed commits, by default GitHub creates a commit message that doesn't follow the commit convention, therefore it's ignored by semantic-release.

But conventional-changelog doesn't ignore this case, so why does semantic-release?

from commit-analyzer.

Yes it totally does. Try to parse the following squashed message from your example:

Squashed commit of the following:

commit 74e6b0087331e5947fd8ba388e75d4d1900dfd44
Author: Jason Walton <[email protected]>
Date:   Thu May 17 11:21:50 2018 -0400

    fix: More fixes

commit b894885ae467975226e81080a35625d247796960
Author: Jason Walton <[email protected]>
Date:   Thu May 17 11:13:25 2018 -0400

    fix: Fix

And you get:

{
   "type":null,
   "scope":null,
   "subject":null,
   "merge":null,
   "header":"Squashed commit of the following:",
   "body":"commit 74e6b0087331e5947fd8ba388e75d4d1900dfd44\nAuthor: Jason Walton <[email protected]>\nDate:   Thu May 17 11:21:50 2018 -0400\n\n    fix: More fixes\n\ncommit b894885ae467975226e81080a35625d247796960\nAuthor: Jason Walton <[email protected]>\nDate:   Thu May 17 11:13:25 2018 -0400\n\n    fix: Fix",
   "footer":null,
   "notes":[

   ],
   "references":[

   ],
   "mentions":[
      "lucid",
      "lucid"
   ],
   "revert":null
}

As you can see it's parsed as a commit that doesn't follow the convention: no type, no scope, no subject. conventional-changelog and all the commit conventions don't do any specific treatment to squashed commit.

from commit-analyzer.

from commit-analyzer.

Try again with that commit message, it won't be included.

You are referring to a previous example that contained BREAKING CHANGE in the body. So the commit was parsed as a breaking change (with no type) and was included because of the breaking change detection only.

from commit-analyzer.

I stand corrected.

from commit-analyzer.

I ended up in this discussion after days of investigation into when semantic-release detects a commit and when it doesn't. This long discussion uncovers the reason why it's implemented like this.

from commit-analyzer.

Hi guys. I've implemented a tiny wrapper for commit-analyzer and release-notes-generator which works with squashed MRs.
You can check it out: semantic-release-unsquash
I've tested it on my project and it works like a charm

from commit-analyzer.