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

Confusion about feat and inconsistency with semver #528

Open
ArwynFr opened this issue May 22, 2023 · 4 comments
Open

Confusion about feat and inconsistency with semver #528

ArwynFr opened this issue May 22, 2023 · 4 comments

Comments

@ArwynFr
Copy link

ArwynFr commented May 22, 2023

There is a confusion/inconsistency with conventional commits and SemVer.
This is especially visible with the confusion around the feat type usage:

Conventional commits:

feat: a commit of the type feat introduces a new feature to the codebase (this correlates with MINOR in Semantic Versioning).

SemVer (emphasis by me):

Minor version Y (x.Y.z | x > 0) MUST be incremented if new, backward compatible functionality is introduced to the public API.

If the feat type is expected to trigger a minor update in SemVer, then the feat CC type should be limited to adding a new feature to the public API of the software. It should not be used when adding a new feature to the implementation which does not update the public interface.

Conventional commits design thought

In my opinion CC 1.0.0 has a design flaw regarding it's relationship to SemVer.

Firstly, conventional commits should have the same constraint on the mandatory existence of a public API.

Using the "main" commit message is a disputable place to handle public API versioning management. Commit messages are expected to document the nature of changes, where the public API is usually a small subpart of the product. If users want to do public API semantic versioning, I think they should rather use some footers, with a well-known footer token such as semver-increment-type for instance. The conventional type can then be used for the implementation change description and conventional commits should list types relevant to this independently from SemVer increment types. I don't think conventional commits was designed for that usage, though (and this should be explicit in the documentation).

If the purpose of conventional commits is pure tooling and using semver as a product versioning scheme, then I don't understand the reasoning behind the current types conventions and especially the usage of the ! operator in conjunction with other types. Conventional commits should rather base itself on the semantic versioning specification and use conventional types based on SemVer increments types such as patch, minor, major. Disputably also pre-release (with a specific format for the pre-release version) and none but that's all.

Also in my opinion the conventions lack a symbol for minor update, such as +, similar to the ! for major updates. There could also be an equivalent of the BREAKING CHANGE: footer, such as NEW FEATURE:. However I think these conventions should not be used in conjunction with types as it overlaps.

@javier-godoy
Copy link

The proposal of the + sign is interesting. We use conventional-commits in order to enforce alignment with SemVer, i.e. that breaking changes target a new major version, and that changes to the public API target a new minor version (details here). We also use some other types in addition to the official fix, feat. Particularly, we use build for changes in external dependencies.

One limitation we found is that it's not possible to convey the level of SemVer change (other than whether it's breaking or not) when upgrading dependencies. For instance, if the upgraded dependency introduces new feature and my codebase extends a class from such dependency, then the codebase inherits a new feature and the commit implies not a patch but a minor change (in terms of your proposal, it would be build+: instead of build:, although CC does not allow custom signs after the type).

With feat however, it's a different story. First, we can use a different type (such as refactor) for cases where the public API is not affected (after all, if the change is not part of the public API, it's just an internal implementation detail). In some cases, we have used scopes to segregate portions of the codebase that do not contribute to the public API (such as tools or executable examples), and we ignore feat commits towards those scopes. Therefore, all the feat commits (with a scope that can contribute to the public API) are effectively minor changes for SemVer.

Using the ! operator in conjunction with other types is also helpful. Some commits (such as those of type refactor or build) can be breaking changes, while others (test, ci, style, docs) are never breaking.

@ArwynFr
Copy link
Author

ArwynFr commented May 22, 2023

For instance, if the upgraded dependency introduces new feature and my codebase extends a class from such dependency, then the codebase inherits a new feature and the commit implies not a patch but a minor change (...)

That is actually incorrect and it was the point I was trying to make.

Updating a dependency is a change in your private codebase, so it should ship as a patch level increment, unless you also introduce changes in your own public API. This is true both for patch-level, minor-level, and even major-level updates in your dependencies.

Semantic versioning is used to version components with a public API and a private implementation. It doesn't care about changes impacting your implementation and only cares about changes in the public API. This is because the purpose of semantic versioning is to manage "dependency hell" by conveying information to dependents on how safe it is to upgrade a dependency. This risk level depends on changes to the public API, not changes to the private implementation, hence the following rule:

Bug fixes not affecting the API increment the patch version, backward compatible API additions/changes increment the minor version, and backward incompatible API changes increment the major version.

In other words, any kind of codebase change can introduce any type of interface change, although some combinations are less probable. This means whenever you change your codebase you also need to convey information on whether your public API was changed, and whether these changes are backward compatible or not. You can use conventional commits types to either convey information on the type of change that was made to the codebase, or the interface, but it cannot do both. It's up to you to decide.

If you decide types are used to categorize interface changes, then it should align with semantic versioning terminology.

If you decide types are used to categorize product changes, then you need an additional information for the interface change type. +/! symbols can be used to that extend for instance. However in that case you should untie the feat type from a minor change and explain the distinction between types and increments.

@piotrminkina
Copy link

Let me join the thread with a somewhat similar problem. There was an interesting discussion at mooltiverse/nyx#315 about whether BREAKING CHANGE or feat!: should bump up the major part in a project version follows Semantic Versioning.

According to the Conventional Commit documentation, every breaking change should raise the version major. According to the Semantic Versioning documentation, this is not true for version 0.y.z. Quoting Semantic Versioning:

Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable.

As @flelli rightly points out, perhaps these ambiguities are due to the meaning of the words CAN and MUST?

This still holds true, but it looks like there's a bit of confusion between the words CAN and MUST. and also what is addressed by SemVer and Conventional Commits. The two get along well, but they're not the same.

Maybe it's worth dispelling such doubts once and for all? Personally, I props for the option in SemVer that specifies that 0.y.z versions, even if they make breaking changes, do not have to conquer the major part in the version. Version 0.y.z is, after all, a place for dynamic application development, where new fiches matter more than the stability of the public API. Semantic Versioning addresses this need very well. Conventional Commits unconditionally enforces the lifting of the major part from the version for each breaking change.

It would be great if there were no doubts as I expressed in the mooltiverse/nyx#315 thread.

@javier-godoy
Copy link

@piotrminkina

According to the Conventional Commit documentation, every breaking change should raise the version major

I couldn't find such wording in https://www.conventionalcommits.org/en/v1.0.0 What is said about breaking changes is:

BREAKING CHANGE: a commit that has a footer BREAKING CHANGE:, or appends a ! after the type/scope, introduces a breaking API change (correlating with MAJOR in Semantic Versioning).

That text says nothing about raising the version major, only that breaking canges correlates with major changes in SemVer (thus it's compatible with not increasing the major version component in 0.x.y versions, if the versioned artifact follows both SemVer and ConventionalCommits)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants