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

Why Do We Recommend improvement As A Type #78

Closed
hutson opened this issue Aug 19, 2018 · 28 comments
Closed

Why Do We Recommend improvement As A Type #78

hutson opened this issue Aug 19, 2018 · 28 comments
Labels
Milestone

Comments

@hutson
Copy link
Contributor

hutson commented Aug 19, 2018

Continuing the conversation from - #66 (comment)

Why do we recommend improvement as a type, and how is improvement better than using other types that signify improvements noticeable to downstream consumers, such as perf?

@damianopetrungaro
Copy link
Member

As you can see I do not agree with the usage of refactor with a BREAKING CHANGE: into it, it's a contradiction imho 😄

I think it's more about lexical meaning of the words.

performance -> it's about performance
refactor -> it's about refactoring and it SHOULD NOT introduce BC
improvement -> it's like a refactor with interal BC.

I am totally open about discussing to remove/change it.

@damianopetrungaro damianopetrungaro added this to the 1.0.0-beta.3 milestone Aug 20, 2018
@Mouvedia
Copy link

In short improvement covers refactors that are not performance improvements.
e.g. readability, dead code removal (for compiled languages), algorithm replacement
Which means that the refactor type will only cover refactors that have a negative or neutral impact.

Would JSDoc comments be covered by docs?
What would be the scope of improvement?
Do you have other examples?
Unless we provide a clear definition of improvement we should drop it and limit ourselves to these.

@damianopetrungaro
Copy link
Member

damianopetrungaro commented Aug 28, 2018

@Mouvedia we should NOT depend on external tools to decide about which type use.

See that the specs only support feat&fix and the usage of improvement is already described in the specs

 We also recommend improvement for commits that improve a current implementation without adding a new feature or fixing a bug

what we can do is add an example and add a section to the FAQ.
Or maybe rephrase the current explanation.

@Mouvedia
Copy link

Mouvedia commented Aug 28, 2018

Recommendations should not conflict with existing conventions that's all I am saying or it should perfectly cover the existing ones. If the lines are blurred it would introduce uncertainty and that's not desired.

@damianopetrungaro
Copy link
Member

@Mouvedia @hbetts any update on this?

@Mouvedia
Copy link

Mouvedia commented Oct 13, 2018

  1. we need use cases (Iv come up with some)
  2. check if these are not already covered by more common types
  3. if so, does having a more generic type like improvement an improvement over multiple types (perf, refactor, etc.)?
  4. if there are cases left after 3, ask ourselves if it is still warranted
  5. if it is, settle on a shortname: Is there an abbr for improvement? #66

@AoDev
Copy link

AoDev commented Jan 11, 2019

Hi,

A long time ago I submitted a question about the meaning of feat, here for context: #26

I just recently discovered the new "improvement" tag recommendation. Maybe it came from my original question(?) That said, to answer @Mouvedia questions:

Since my own question, I have been using the tag "imp", as "improvement", in all my projects.
The typical usage of this tag is when I improve the internal libraries of my apps. Libraries that could well be standalone and published as separate packages, but not worth the hassle until a second project requires them.

Cases where I do this:

  • Any added functionality to "internal" libraries.
  • New UI framework / generic UI components
  • New features under feature flag: until the end-user sees it, it is not "feat"
  • Subtle improvement in UI that is not worth considering a "feat", nor a "fix"

Example:

asyncUtils.js (an already existing collection of utilities to manage async code)

I find myself needing a new async utility to create a new feature, that would lead to two commits:

  • imp(asyncUtils): added new function...
  • feat(app): new feature that uses the new async utility

In summary: any added functionality to the "internals" of the app or library that is irrelevant to the end-user. They don't qualify as feat nor refactor.

I have been doing this for one year now. It works pretty well because I can generate changelog for end-users from feat and fix while keeping track of internal improvements and better split commits. And I find it funny, it makes me think of fairies. :P

@Mouvedia
Copy link

Mouvedia commented Jan 11, 2019

Alright here's my take based on your description:

  • doesn't represent immediate changes for the user
  • doesn't improve performance by itself
  • doesn't modify an existing feature
  • adds an inactive internal functionality
  • always represents a step towards another type (feat, refactor, perf, etc.)

@AoDev Anything to add?

@AoDev
Copy link

AoDev commented Jan 11, 2019

@Mouvedia Yes that's a good definition of what I have in my head. Especially your last point is spot on I would say.

@Mouvedia
Copy link

This shit is goddamn narrow in scope. Would internal be more encompassing and easier to describe?

@AoDev
Copy link

AoDev commented Jan 11, 2019

I don't think it's a "goddamn narrow in scope".

If I look at my last project (medium size SPA)

Commits count go like this:
imp: 86
refactor: 72
feat: 70
fix: 41

It's the most used tag. Obviously it depends on app architecture. Personally I set a clear line between business logic (would lead to feat) and generic stuff (leads to imp).

@damianopetrungaro
Copy link
Member

Closing this due to inactivity.

Feel free to re-open it if needed! 😄

@karol-depka
Copy link

Please use improve; much shorter than improvement while still an englishword* with its own clear meaning. Please don't play this game of crazy abbreviations. Imp, for example, can stand for so many different things, including implementation, it's scr*wing with your brain. Shock Minimization Principle. Imagine someone new coming to a project and seeing a bunch of imp (implementation?) or improv (improvised solution?); must seem crazy and confusing at the beginning. We should reduce barriers and confusions, not create new ones. Please upvote if You agree! :) Common sense FTW!
This whole over-shortening game is a bit like Perl's Akka's crazy special-chars operators... ...considered harmful.

Thank You for Your work on trying to standardize commit messages :).

@rcdailey
Copy link

rcdailey commented May 27, 2021

Why not change? As far as this discussion goes, my suggestion is relevant for me personally because:

  1. I follow keep a changelog which has a section for "Changed"
  2. The specific change is important enough for the user to know about, but not big enough to be called a feature. My application parses JSON data. When new optional fields are added to the JSON, I like to add support for them. I don't want to bump my version number's 2nd component. I'd rather release that as 1.5.1 instead of 1.6.0 (going from 1.5.0).

Either way, in my case, I personally will go with:

change(scope): support new field in json payload

@damianopetrungaro
Copy link
Member

@rcdailey to me change is too vague, it may include any other scope as well which kinda conflict with the conventional commit purpose.

@anburocky3
Copy link

So do you guys decided? 🤣 Looks like multiple discussion going around and what did you guys decided at the end?

I personally feel, improve: is okay, other terms like imp:, im makes confusions

@SrBrahma
Copy link

SrBrahma commented Jun 5, 2022

I have a login system. The loading modal was only being shown after some steps, where it could be shown earlier. I improved it and it's now being shown properly.

It wasn't a bug so it isn't a fix. It isn't a feat because it already existed.

I ain't yet too familiar with Conventional Commits, so I googled "conventional commits improvement" and got here.

+1 for improve:

@melsabagh
Copy link

Hate to flog a dead horse, but what's wrong with enh: for enhancement/improvement?

@martin-braun
Copy link

martin-braun commented Jan 25, 2023

I think we should reopen this for cases like making a few things better in the UI so it becomes easier for the user, or when updating translations to give better information to the user. It's not really a feature, because it doesn't bring something new to the table. It's also not fixing something, it's just improving something.

enh, enhance, impr, improve, whatever will it be. A common standard is what I like and would really love to see.

@tobias-edwards
Copy link

Sounds like improvement is being suggested as a minor feat or fix - a tweak, if you will.

Or it's being suggested as the type that describes smaller commits that forms part of a feat or fix pull request.

Do we want/need this granularity? Commit messages should be easy to write, I don't want to enter a state of analysis paralysis everytime I commit something small:

Is this an improvement? All features and fixes are improvements. Is this big enough to be a feature/fix? Etc.

@AoDev
Copy link

AoDev commented Jan 25, 2023

Years have passed since I got interested in this question. I've changed my strategy since then and wanted to share it here for anyone interested.

I combine the chore tag with the others.

For example a fix in some internal framework becomes

chore: fix(something): ... or just chore: fix: description

Another example would be an improvement in the UI but not worth being listed as a public new feature in a changelog. (example in previous comments)

chore: feat(login): bigger button for better UX

Basically prefixing with chore what would be the actual thing if it were public and still following the guidelines of conventional commits. (Angular flavour)

If there is ever a new official tag recommended, great :)

@melsabagh
Copy link

@AoDev Does chore get mapped to anything when bumping? The tools I have tried don't bump on chore changes.

@SrBrahma
Copy link

Sounds like improvement is being suggested as a minor feat or fix - a tweak, if you will.

I actually like tweak

@Mouvedia
Copy link

1: a small change or adjustment

👍 for tweak

@martin-braun
Copy link

tweak would be a great addition to the open standard.

@SrBrahma
Copy link

@damianopetrungaro can you reopen this due to this good (and quite unexpected) tweak suggestion? Thanks!

@DoctorDerek
Copy link

They're features (feat) unless you've identified a clear usability (user experience / UX) or accessibility (a11y) defect, in which case the enhancement is a bugfix (fix).

@martin-braun
Copy link

@DoctorDerek I let that sink in for a while and I think you are right. If you have a tweak ask yourself, does it solve a mis-alignment, then it's a fix, does it add something new in a minor fashion, then it's a minor feat.

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

No branches or pull requests