search

vivaxy / vscode-conventional-commits

πŸ’¬Conventional Commits for VSCode.
https://marketplace.visualstudio.com/items?itemName=vivaxy.vscode-conventional-commits
MIT License
322 stars 35 forks source link

[FEAT] automatically prepend `BREAKING CHANGE: ` #39

Open TeFiLeDo opened 3 years ago

TeFiLeDo commented 3 years ago

Is your feature request related to a problem? Please describe. While the extension explicitly prompts the user to enter any breaking changes, it expects you to prepend BREAKING CHANGE: manually. In my opinion this is a cumbersome task that should be automated.

Describe the solution you'd like A possible solution would be to simply prepend BREAKING CHANGE: automatically, if the user writes something when prompted for breaking changes.

Describe alternatives you've considered A simpler, but in my opinion inferior, solution would be to no longer ask for a text value for breaking changes. This could be replaced by asking whether or not there were breaking changes and then adding an exclamation mark to the description when there are.

yi-Xu-0100 commented 3 years ago

πŸ‘‹ @TeFiLeDo I make a PR to solve this issue, try to use the beta extension - vscode-conventional-commits-330916b.vsix !

But I think it still needs some discussion with the author, you can subscribe to the PR (#96) to trace the progress about the solution. If you upgrade the extension, feel free to ask me when you have any questions. πŸ˜€

vivaxy commented 3 years ago

πŸ‘‹ @TeFiLeDo Thank you for your suggestion. πŸ˜ƒ @yi-Xu-0100 Thank you for your contribution.

I've some concerns about this feature:

Conventional Commits Specification had another way to commit a breaking change:

Commits MUST be prefixed with a type, which consists of a noun, feat, fix, etc., followed by the OPTIONAL scope, OPTIONAL !, and REQUIRED terminal colon and space.

Quote from Conventional Commits Specification.

How can we make the extension follows the specification? What should be the default commit behavior? How can we not break the user experience now?

yi-Xu-0100 commented 3 years ago
  1. Breaking changes MUST be indicated in the type/scope prefix of a commit, or as an entry in the footer.
  2. If included as a footer, a breaking change MUST consist of the uppercase text BREAKING CHANGE, followed by a colon, space, and description, e.g., BREAKING CHANGE: environment variables now take precedence over config files.
  3. If included in the type/scope prefix, breaking changes MUST be indicated by a ! immediately before the :. If ! is used, BREAKING CHANGE: MAY be omitted from the footer section, and the commit description SHALL be used to describe the breaking change.

@vivaxy The specification described that the breaking change must be indicated.

Maybe I can add a configuration named promptBreakingChange, which used to enable the prompt of BREAKING CHANGE(default false). And add a feature to detect BREAKING CHANGE: or BREAKING-CHANGE: in the subject, which will add a ! in prefix to point out the BREAKING CHANGE in commit.

The feature will not break but improve the user experience now. The breaking change should be less so it is appropriate for the prompt to be disabled by default.

vivaxy commented 3 years ago

@yi-Xu-0100 Yes, the breaking change must be indicated according to the specification. πŸ˜ƒ But there are 2 ways of adding a breaking change. What I am thinking is about the interaction design to add a breaking change:

  1. Some of us would like to use a traditional way: the BREAKING CHANGE: in the footer written by hand.
  2. Some of us would like to prompt for a breaking change message. Like we have implemented in https://github.com/vivaxy/vscode-conventional-commits/pull/96.
  3. Some of us would like to prompt for a breaking change before type/scope.

I'd like to have more discussion about those interaction design.

Is there a way to support those 3 ways? And anything missing here? πŸ€”

yi-Xu-0100 commented 3 years ago

@vivaxy Although the specification did not make it as "MUST", I still think the ! should not be missing because it will see clearly in log when we see the title of the commit message.

Add a feature to detect BREAKING CHANGE: or BREAKING-CHANGE: in the subject, which will add a ! in prefix to point out the BREAKING CHANGE in commit.

So I prefer to add the above feature. Maybe add a configuration to enable the feature?

  1. Add a feature to detect BREAKING CHANGE: or BREAKING-CHANGE: in the subject, which will add a ! in prefix to point out the BREAKING CHANGE in commit. It is disabled as default. detectBreakingChange is a configuration.
  2. Add a feature to add a prompt about BREAKING CHANGE. It is disabled as default. promptBreakingChange is a configuration.

The two configurations will do well in three ways and also do well in them combined.

vivaxy commented 3 years ago

@yi-Xu-0100 What about the empty breaking change message? Like:

  1. Some of us would like to prompt for a breaking change before type/scope.

If an empty breaking change message is entered, will the extension add BREAKING CHANGE: in footer or add ! before type/scope? BREAKING CHANGE: with empty message in footer seems strange.

yi-Xu-0100 commented 3 years ago

@vivaxy You mean that Some of us want to only add ! but not add the BREAKING CHANGE:?

Maybe the promptBreakingChange can make the prompt have three choices, one is !, the second is BREAKING CHANGE:, the third is BREAKING-CHANGE:.

For the !, It will add ! and jump to the footer input. For the BREAKING CHANGE: and BREAKING-CHANGE:, it will show the input box for adding context after the prefix. If you want to add !, enable the detectBreakingChange will helpful.

vivaxy commented 3 years ago

@yi-Xu-0100

@vivaxy You mean that Some of us want to only add ! but not add the BREAKING CHANGE:?

Yes.

the third is BREAKING-CHANGE:

What is the BREAKING-CHANGE: use case? Why not just BREAKING CHANGE:?

For the !, It will add ! and jump to the footer input.

Maybe we can prompt for ! before type to make the prompt procedures follow the writing procedures?

For the BREAKING CHANGE: and BREAKING-CHANGE:, it will show the input box for adding context after the prefix. If you want to add !, enable the detectBreakingChange will helpful.

That's nice.

yi-Xu-0100 commented 3 years ago

@vivaxy

  1. A footer’s token MUST use - in place of whitespace characters, e.g., Acked-by (this helps differentiate the footer section from a multi-paragraph body). An exception is made for BREAKING CHANGE, which MAY also be used as a token.

The specification point it out that BREAKING-CHANGE: may be used as a token in the footer.

  1. BREAKING-CHANGE MUST be synonymous with BREAKING CHANGE, when used as a token in the footer.

But I don't know whether the BREAKING CHANGE should add or not when the BREAKING-CHANGE was used to be a token in the footer. πŸ€”

vivaxy commented 3 years ago

@yi-Xu-0100

The specification point it out that BREAKING-CHANGE: may be used as a token in the footer.

That's true. Ref: https://github.com/conventional-commits/parser/blob/main/lib/parser.js#L326 and

  1. BREAKING-CHANGE MUST be synonymous with BREAKING CHANGE, when used as a token in the footer.

But I don't know whether the BREAKING CHANGE should add or not when the BREAKING-CHANGE was used to be a token in the footer. πŸ€”

BREAKING CHANGE and BREAKING-CHANGE seems to be the same. Maybe we can keep BREAKING-CHANGE when BREAKING-CHANGE.

yi-Xu-0100 commented 3 years ago

@vivaxy You mean that:

  1. Add BREAKING-CHANGE in footer when BREAKING-CHANGE but not add BREAKING CHANGE in body? (Also not add ! in prefix and will add ! in prefix when detectBreakingChange is true)
  2. Add BREAKING CHANGE in body when BREAKING CHANGE but not add BREAKING-CHANGE in footer? (Also not add ! in prefix and will add ! in prefix when detectBreakingChange is true)
  3. Add ! in prefix when ! but not add BREAKING CHANGE or BREAKING-CHANGE?
vivaxy commented 3 years ago

@yi-Xu-0100

  1. If the users type BREAKING CHANGE: or BREAKING-CHANGE: in footer, we should do nothing.
    1. BREAKING CHANGE: or BREAKING-CHANGE: should appears only in footer. If we do detection, we should try to only detect the token in footer.
    2. BREAKING CHANGE:, BREAKING-CHANGE: and ! have the same meaning. We don't have to add more BREAKING CHANGE:, BREAKING-CHANGE: or ! in commit messages if the commit message already contains any breaking change tokens. If we do add , be aware of the commmitlint length rules.
  2. I think the issue is about how to write a breaking change message. So adding a prompt could be the solution. Is there any better solutions? And how to add a prompt while we are keeping the current experience, supporting commitlint and e.t.c.?
yi-Xu-0100 commented 3 years ago

@vivaxy I mean that a prompt about how to add the BREAKING CHANGE. In my design, it should be similar to the selection prompt like scope, which has four choices.

  1. no BREAKING CHANGE.
  2. !： add ! into prefix, and it needs description BREAKING CHANGE in description or body by hand.
  3. BREAKING CHANGE: open an input box for the BREAKING CHANGE: message, this message will append after the body and before the footer.
  4. BREAKING-CHANGE: open an input box for the BREAKING-CHANGE: message, this message will prepend into the footer section.

It will have two configurations, the one is detectBreakingChange, the other is promptBreakingChange, they all default to false. If the detectBreakingChange set to true, it will add ! into the prefix, if there are any BREAKING CHANGE or BREAKING-CHANGE in the commit message. If the promptBreakingChange set to true, it will add a prompt after the scope prompt and before the gitmoji prompt. And the prompt of BREAKING CHANGE has the four choices described above.

For users before the version, two configurations default to false, which leads to no influence on them.

About the rules about the commitlint, I need to figure out this extension how to deal with the rules and try to fit it well.

<type>[optional scope]: <description>

[optional body]

[BREAKING CHANGE: message] (if choose the `BREAKING CHANGE`, it will add it here.)

[BREAKING-CHANGE: message] (if choose the `BREAKING-CHANGE`, it will add it here.)
[optional footer(s)]
vivaxy commented 3 years ago

@yi-Xu-0100

Q1: The Options.

In my design, it should be similar to the selection prompt like scope, which has four choices.

I think the 4 choices are mixing commit messages options with user preferences.

Commit message options should prompt in each commit with following options:

  1. This commit is a breaking change.
    1. The breaking change has more information. Should be with BREAKING CHANGE: or BREAKING-CHANGE: in footer.
    2. The breaking change has no other information. Should be ! after scope and before description.
  2. This commit is not a breaking change.

User preferences should be something shared between projects or users. They are set in config files. They have following options:

  1. Whether to prompt for a breaking change or to write the breaking change in footer section.
  2. Preferring BREAKING CHANGE: over BREAKING-CHANGE: or vice versa.

What do you think?

Q2: Why detectBreakingChange?

I think !, BREAKING CHANGE:(or BREAKING-CHANGE:) is the same thing. Why will we append ! when BREAKING CHANGE:(or BREAKING-CHANGE:) is in footer?

yi-Xu-0100 commented 3 years ago

@vivaxy

For the option.

Commit message options should prompt in each commit with following options:

  1. This commit is a breaking change.
    1. The breaking change has more information. Should be with BREAKING CHANGE: or BREAKING-CHANGE: in footer.
    2. The breaking change has no other information. Should be ! after scope and before description.
  2. This commit is not a breaking change.

I think this will lead to one more page choosing, and the extra selection page will take unnecessary time. If there is not a breaking change (for the most time), I need to type enter to skip. If there is a breaking change, I can choose a way to add a prompt into the message. It can be three options.

  1. not a breaking change.
  2. The breaking change has no other information. Should be ! after scope and before description.
  3. The breaking change has more information. Should be with BREAKING CHANGE: in body or BREAKING-CHANGE: infooter(by a new configuration breakingChangePattern).

Add a new configuration breakingChangePattern for the pattern of BREAKING CHANGE (BREAKING CHANGE in body or BREAKING-CHANGE in footer). Add a new configuration promptBreakingChange for whether add a prompt for user.

The scope page has the same logic. Now it has a promptScopes for whether add a prompt for user. And the page have a three option. πŸ˜€

For the new configuration breakingChangePattern, I think it is not necessary. Because the BREAKING CHANGE is not a usually used option, the configuration for it will lead to heavy. And add an option to the selection page will not add more time for selecting when really need to add a BREAKING CHANGE. πŸ€” In the most time, I think I want one enter to skip the prompt not two enter if I enable the prompt. For the selection I used less, I think the more choices for it also is flexible than a configuration. 😁

What do you think about the usage frequency of the BREAKING CHANGE?

For the detectBreakingChange.

In my use case, I always read the commit message log in the short mode or from the GitHub page. It will only show the <type>[optional scope]: <description> so that I can see more logs. If there is BREAKING CHANGE in the body or the footer but missing ! in the prefix, I will not find it immediately. So I prefer the ! when having a BREAKING CHANGE. πŸ™‚

When having a BREAKING CHANGE, but it needs a clear description, I also need to add it into the body. But I did not want to drop the ! from the prefix. πŸ˜…

The specification did not point it to a conflict, furthermore, there is an example for it in the specification (commit message with both ! and BREAKING CHANGE footer). I think that to add a clear description about the BREAKING CHANGE in the body meanwhile to add a ! in the prefix will improve log reading experience. 😜

vivaxy commented 3 years ago

@yi-Xu-0100 I've 2 more questions:

Q1:

Why is BREAKING CHANGE: in body happens? I think it can only appears in footer.

Q2:

According to your suggestion:

It can be three options.

  1. not a breaking change.
  2. The breaking change has no other information. Should be ! after scope and before description.
  3. The breaking change has more information. Should be with BREAKING CHANGE: in body or BREAKING-CHANGE: infooter(by a new configuration breakingChangePattern).

I think there will be 1 or 2 more prompts when promptBreakingChange is on:

  1. Select from NOT A BREAKING CHANGE, BREAKING CHANGE WITHOUT MESSAGE or BREAKING CHANGE WITH MESSAGE
  2. If BREAKING CHANGE WITH MESSAGE selected, enter the breaking change message.

Am I right?

yi-Xu-0100 commented 3 years ago

@vivaxy

For the body I described.

I misunderstand the specification, so the style of the commit message is like below? There is no blank line between the footer?

<type>[optional scope]: <description>

[optional body]

[BREAKING CHANGE: message] or [BREAKING-CHANGE: message]
[optional footer(s)]

For the description of the prompt page.

Right! Is there necessary to set the pattern of the BREAKING CHANGE?

I think that if we need to add a configuration for the pattern, Could we allow setting the configuration for more flexibility, I think it can be three choices for the pattern, BREAKING CHANGE, BREAKING-CHANGE and both. What do you think? Which of them should be the default? πŸ˜€

vivaxy commented 3 years ago

@yi-Xu-0100 

<type>[optional scope]: <description>

[optional body]

[BREAKING CHANGE: message] or [BREAKING-CHANGE: message]
[optional footer(s)]

Yes, I think this is the specification.

Is there necessary to set the pattern of the BREAKING CHANGE?

Yes, I think it's reasonable.

I think it can be three choices for the pattern, BREAKING CHANGE, BREAKING-CHANGE and both.

In what situation is the both used?

In my use case, I always read the commit message log in the short mode or from the GitHub page. It will only show the <type>[optional scope]: <description> so that I can see more logs. If there is BREAKING CHANGE in the body or the footer but missing ! in the prefix, I will not find it immediately. So I prefer the ! when having a BREAKING CHANGE. πŸ™‚

Thank you for you explanation. I think it's reasonable to have the detectBreakingChange option.

yi-Xu-0100 commented 3 years ago

@vivaxy I really don't know the situation about the BREAKING-CHANGE as the token in the footer, but I did not want to miss it when I figure out why it exists and want to use it in the future. πŸ˜‹

For the reason that BREAKING CHANGE has a low usage frequency, The four options also not bother me, and the configuration with the both will not consume more time for general committing. πŸ˜€

When the pattern is BREAKING CHANGE or BREAKING-CHANGE, the selection page can be three options. When the pattern is the both, the selection page can be four options in case of future usage without setting more. 😁

c-vetter commented 3 years ago

Hello there, I came across https://www.conventionalcommits.org today and that pointed me to this extension. I like the idea very much.

When I tried out the extension, the behavior of the breaking-change-prompt appeared to me as a bug because it does not seem to follow the specs. Looking to open a bug report, I found this issue, so I will add my two cents here instead.

Firstly, as stated, the current behavior seems like a bug. The reason for that is that the prompt asks me for the breaking changes, not for an additional trailer. That really implies that the extension will take care of the structural requirements connected to breaking changes.

Regarding how this should work, I'd like to propose a different strategy than @vivaxy and @yi-Xu-0100 have discussed so far. The prompt should really have only one non-optional page asking for the description of any breaking change:

This strategy reduces necessary interaction to a bare minimum:

The ! for a breaking commit with a description could be optional but should default to adding it because that maximizes visibility as @yi-Xu-0100 explained.

Whether to use BREAKING CHANGE: or BREAKING-CHANGE: should be an option defaulting to BREAKING CHANGE: because that is the main use case in the spec. The way I read the spec, BREAKING-CHANGE: is not an important use-case but really only mentioned in order to allow using the dashed style that the other footer tokens use, probably to facilitate some simpler tools.

Regarding not breaking the current behavior, my suggestion is to either accept this as a bug fix or make the whole thing opt-in and mark the current behavior as deprecated and prominently pointing to this improvement. At v2, this really should be the only way.

Looking forward to your feedback on this. @vivaxy thanks for this extension! πŸ™‚

seloner commented 3 years ago

I agree that the extesion shoud prepend the BREAKING CHANGE: I had a version issue because standard-version didnt understand the breaking change

djdembeck commented 3 years ago

I like @rasenplanscher idea. I also thought this was a bug, as I have been adding breaking changes but standard-version wasn't picking them up as such.

emiliosheinz commented 1 year ago

Any updates on that? I see that there is an open pull request that was never merged.

yi-Xu-0100 commented 1 year ago

πŸ‘‹ @emiliosheinz, Thank you for your concern, but due to a lack of available development time, this feature has been temporarily put on hold. I would greatly appreciate it if someone could contribute a pull request for it.