-
Notifications
You must be signed in to change notification settings - Fork 548
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
Should feat
cover changes to existing features?
#294
Comments
@bcoe what do you think? |
Let's start putting the 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 Dropping a feature will always be a When I do this, I typically call it 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 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 |
Two random commits I just looked up:
|
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.
I think this fits well within the meaning of
I think the first example is is a clear example of a |
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).
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. |
If we take semver's
we can see that MAJOR and MINOR cover "add functionality" and "API changes". Both MAJOR and MINOR can be of type 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"). |
Why can't just add another type |
For me "change" mean any change. Not specifically a "new" or a It 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
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. 😆 In general, I 💯 agree that this text isn't good enough. Maybe "... when a commit changes a feature ...", or why not just include the "correlates to SemVer |
I wonder, did the meeting happen and if so, what was the outcome with regards to this issue? |
If I think it's wiser to use another keyword such as |
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
into something that covers "changes or drops an existing feature"
The text was updated successfully, but these errors were encountered: