Adopt commitlint and/or semantic-release

[amanda-mitchell]

@bryanrsmith recently introduced me to commitlint and semantic-release, which both greatly simplify the process of issuing releases and release notes.

By contrast, the release policy in this repo is to manually update Version History both when making changes and when issuing a new release. (Aside: I'm not even entirely certain how new releases happen. I suspect that it has something to do with manually changing a version number somewhere, but I'm not confident about this.)

I'd like to adopt something similar for FaithlifeAnalyzers (and possibly other Faithlife C# projects), but there don't seem to be mature alternatives to commitlint and semantic-release in the .net ecosystem.

I did, however, find this article, which describes using semantic-release directly from cake.

Are there any strong objections to trying this out? (particularly from active contributors: @bgrainger, @ejball, @ddunkin, @StephenCleary)

In addition to configuring semantic-release, I'd also want to reconfigure the build to run commitlint against all PR commit messages, since semantic-release isn't reliable without parsable commit messages.

This last bit is perhaps the largest potential downside: in the node ecosystem, there are tools like husky that make it easy to install precommit hooks that run commitlint at commit time—without this, some developers might miss the need to follow the commit message guidelines until the PR build output informs them of violations—resulting in the need for a possibly painful rebase.

[bgrainger]

I'm not immediately a fan of writing commit messages for machines, not for other humans. I'd like to have more of a discussion about this.

[ddunkin]

I find automatically generated change logs hard to read when used as the primary documentation of what's changed in a version. A single high-level change could have multiple commits. There's noise from internal refactoring, formatting and style changes, tests, etc. that doesn't affect the API.

I do find a detailed change log helpful as a supplement to a summary of changes.

I've found myself wanting to scope my commit messages, e.g. "build" or "docs", but I also think the "feat(foo):"-style prefix makes it harder to read a list of commits. It's ugly. Maybe one just needs to acclimate to it?

Mapping "fix" to "patch" and "feat" to "minor" doesn't always work. Sometimes a fix requires an API change, and sometimes a feature doesn't change the API, e.g. an improvement to an existing feature. Is there a way to deal with that with semantic-release?

[amanda-mitchell]

writing commit messages for machines, not for other humans

I think this may be overstating the case. Commitlint enforces the Conventional Commits standard, which primarily consists of a handful of prefixes on commit messages as well as a standard way to signal the existence of breaking changes.

I initially found the type and scope prefixes a bit odd, but I've found that it tends to enhance the readability of the commit messages.

[bryanrsmith]

There are definitely tradeoffs. I've found it to work pretty well, in practice, and the things I thought would annoy me about it weren't such an issue.

There's noise from internal refactoring, formatting and style changes, tests, etc. that doesn't affect the API.

refactor, chore, style changes don't trigger releases and aren't included in release notes. You can configure this, if you want.

Mapping "fix" to "patch" and "feat" to "minor" doesn't always work. Sometimes a fix requires an API change, and sometimes a feature doesn't change the API, e.g. an improvement to an existing feature.

A fix requiring a breaking change would use the fix type, with a BREAKING CHANGE addition, and would trigger a major version change. You'd have to make a judgement call on fix that requires a non-breaking API, but labeling it as feat would probably be appropriate. I agree that the scope of the feat type is broad; I believe the name of the type was intended more to get developers out of the habit of thinking about version numbers and more in the habit of thinking about the type of change being made.

[amanda-mitchell]

Mapping "fix" to "patch" and "feat" to "minor" doesn't always work. Sometimes a fix requires an API change, and sometimes a feature doesn't change the API, e.g. an improvement to an existing feature. Is there a way to deal with that with semantic-release?

Under Conventional Commits, breaking changes are signaled by included a final paragraph that starts with BREAKING CHANGE: and a description of the change. This can be combined with any of the other commit types and overrides the normal version handling.

sometimes a feature doesn't change the API

Under SemVer, this should still be a minor version update, so the type prefix feat would still apply.

[ddunkin]

sometimes a feature doesn't change the API

Under SemVer, this should still be a minor version update,

"[Minor version] MAY be incremented if substantial new functionality or improvements are introduced within the private code."
https://semver.org/#spec-item-7

[bryanrsmith]

The only thing that I've really disliked about semantic-release is making mistakes in commit messages. I've occasionally merged a PR from a contributor that neglected to include a BREAKING_CHANGE note, or otherwise got the change type wrong. This is difficult to correct since a release is made immediately upon merging the change. It could be mitigated somewhat by running semantic-release on a release branch, giving maintainers an opportunity to review changes before merging to the branch that triggers a release.

(I have no opinion on whether or not it should be used in this project; just adding context)

[bgrainger]

Despite arrogating the name "Conventional Commits", this doesn't match historical git conventions. It also seems incompatible with git command-line tools that automatically generate Merge ... and Revert ... commit messages.

It takes up a lot of the recommended 72 characters with "noise" and reduces the available space for a meaningful commit description.

It feels unnecessary in a strongly-typed language with reflection where you can (and we have!) build a tool to diff published APIs and figure most of this out automatically.

It makes determinations that may be wrong (e.g., determining the severity of the change) immutable by embedding them in git commit messages. (Assuming you don't force push.)

[amanda-mitchell]

It could be mitigated somewhat by running semantic-release on a release branch, giving maintainers an opportunity to review changes before merging to the branch that triggers a release.

Another option would be to configure the PR builder to post a comment back to the PR indicating what version would be published after merging.

[amanda-mitchell]

It feels unnecessary in a strongly-typed language with reflection where you can (and we have!) build a tool to diff published APIs and figure most of this out automatically.

I don't see this as an either/or proposition. The commit message signals developer intent, whereas the use of reflection can be used as a checksum of sorts as to whether that intent is compatible with what was actually done.

As it stands now, we have no established conventions for a developer to communicate what kind of change he or she intended to make (in terms of major/minor/patch). Adding such a convention (even if it isn't this one, exactly) would simplify some code review tasks—and adding a convention in a way that is machine-readable would allow tooling to perform some of the verification.

[ddunkin]

I object to automatically releasing when code is pushed or merged to master. There are plenty of times I've had multiple change sets submitted in different PRs to make reviews easier, but I want them all in the same release. Releasing should be an intentional process.

[amanda-mitchell]

I object to automatically releasing when code is pushed or merged to master.

I don't have strong feelings about this—I'd be happy to establish a branch other than master that represents "code for immediate release".

[ddunkin]

It makes determinations that may be wrong (e.g., determining the severity of the change) immutable

I was going to say "It makes it easier to lie," but that's a nicer way of putting it. :)