conventionalcommits.org
conventionalcommits.org copied to clipboard
conventional-commits
Should `feat` cover changes to existing features?
Changing or dropping an existing feature (and as such changing the public API) is not covered by the core spec currently as far as I'm aware. This leads to confusion around what type to use in this case. Personally I like the definition "feat describes a change related to the public API"
I'd propose to change
The type feat MUST be used when a commit adds a new feature to your application or library.
into something that covers "changes or drops an existing feature"
Let's start putting the meeting-agenda label on issues we think we'd like to discuss on a monthly call.
I opened an issue for us to have a first meeting -- my hope is that getting a few collaborators in a room together, we can better evolve the spec.
I do think this description of feat is limited, but I think your above example has only one case not covered: dropping a feature. Changing is either a fix to the existing feature or a new feat on top of the existing feature.
Dropping a feature will always be a ! or BREAKING CHANGE, but it is IMO explicitly not when a commit adds a new feature to your application or library..
When I do this, I typically call it fix!: removed feat x because I always try to do a feat!: deprecate feat x first, and so the fix just removes the dep and method/property/behavior. I can see how this might not actually solve the problem for many, but I see a deprecation warning as a breaking feature, the following removal is also breaking but a fix since it is already deprecated.
Do you have any other prior art or approaches projects take for this kind of commit?
Personally I'd say that changing a feature is not a fix. In my mind a fix is a bug fix with the primary task to restore intended feature behavior, which might encompass changing a feature but probably shouldn't.
Yeah, modifying or dropping existing feature behavior is probably almost always breaking.
There's also changing an existing feature by adding new behavior in a backwards-compatible way. Depending on how the feat spec is interpreted, it could mean that it doesn't cover this, since "changing" / "modifying" / "extending" isn't mentioned.
Do you have any other prior art or approaches projects take for this kind of commit?
Two random commits I just looked up:
- https://github.com/yargs/yargs/commit/8639 (drops a feature)
- https://github.com/yargs/yargs/commit/89456d994216466da2be929d81d3fb936005ae96 (changes a feature)
I am not sure your distinction matters to semver, tooling, or users reading the message. I am also not sure what type of change you can make which is not fixing a bug (restoring old behavior or fixing unintended behavior) and also not an new feature or sub-feature. If you have a concrete example that might help me understand.
There's also changing an existing feature by adding new behavior in a backwards-compatible way.
I think this fits well within the meaning of feat. Remember this is about "semverness", and to semver (and my interpretation) all of those are the same ("changing" / "modifying" / "extending").
Two random commits I just looked up:
I think the first example is is a clear example of a feat, as it adds explicit checking. To me, that is enough metadata covered by the spec to indicate the correct meaning to me. Is there something you think we could change/add which would make this kind of change meaningfully different from feat?
If you have a concrete example that might help me understand.
I think the second commit I looked up covers this. The commit changes the behavior of an existing feature (it doesn't fix or add something).
I think the first example is is a clear example of a feat, as it adds explicit checking.
That's what happened internally, yes. It's not what happened in terms of "public features" - what happened for consumers with the first commit is that a feature (having support for Node 8) was dropped.
Remember this is about "semverness"
If we take semver's
MAJOR version when you make incompatible API changes, MINOR version when you add functionality in a backwards compatible manner, and PATCH version when you make backwards compatible bug fixes.
we can see that MAJOR and MINOR cover "add functionality" and "API changes". Both MAJOR and MINOR can be of type feat, and hence I'd like to see feat cover "API changes" as well.
And I guess "add functionality" instead of "add feature" helps to better grasp that functionality can also be added to existing features (and hence changing an existing feature, which, yes, technically is "add feature to feature").
@wesleytodd Changing is either a
fixto the existing feature or a newfeaton top of the existing feature.
For me "change" mean any change. Not specifically a "new" or a fix. But anyway, I really agree with all of your comments.
@stoically
It ~~clearly~~ shadowy states it correlates to SemVer MINOR, from where we see that the most important and distinctive factor is... is it breaking or not, both for minor and major. So, feat can be anything "new; backward-compatible; including a fix, and so any changes to private / internal APIs".
That said, @stoically, your question should be "Is that 'drop' touch public APIs, does it break something?". If not, if it's dropping/changing private/internal APIs, then... no, it's not breaking, so it's can be labeled as feat or whatever.
what happened for consumers with the first commit is that a feature (having support for Node XXX) was dropped.
Technically, it's not a real/actual feature anymore at the time of "dropping" it, it's a burden - for both parties, maintainers and users. So in reality, removing it is "a feature" both in terms of SemVer and CC. :laughing:
In general, I :100: agree that this text isn't good enough. Maybe "... when a commit changes a feature ...", or why not just include the "correlates to SemVer MINOR" in the spec section, because now it's somewhere above in the summary just as some recommendation or I don't know what.
I wonder, did the meeting happen and if so, what was the outcome with regards to this issue?
If feat is used for changes and/or removal of features, then won't that make it harder to generate change logs automatically?
I think it's wiser to use another keyword such as cut or remove. And I guess there's nothing stopping anyone right now from using cut along with BREAKING CHANGE or !, right?
