Giter VIP home page Giter VIP logo

Comments (36)

stevemao avatar stevemao commented on May 26, 2024 4

conventional-changelog currently implements revert exactly the same way as the angular docs.

Revert

If the commit reverts a previous commit, it should begin with revert: , followed by the header of the reverted commit. In the body it should say: This reverts commit <hash>., where the hash is the SHA of the commit being reverted.

from conventionalcommits.org.

hutson avatar hutson commented on May 26, 2024 3

My two cents would be to simply document what git already does. As noted by @damianopetrungaro using git revert and keeping it's auto-generated message is easy. Doing anything else just adds friction and a greater chance of not adhering to the convention.

because Remove birthday is not a feature itself, it may be part of a fix (but this means it's not reverted).

In my experience a revert is always a fix. Even if it the commit technically adds a feature, it's only because the removal of that feature was unintended (it essentially introduced a bug).

However we really should document how to handle revert commits.

from conventionalcommits.org.

Mouvedia avatar Mouvedia commented on May 26, 2024 1

@hbetts that doesn't cover my current usage of the revert type which is a partial revert using git commit. The intent is a revert but granular: https://stackoverflow.com/q/45267653

from conventionalcommits.org.

Mouvedia avatar Mouvedia commented on May 26, 2024 1

IMHO the short hash of the original commit should be mandatory. Since you allow custom types Ill continue to use the revert type. I was just enquiring as to why it was omitted.

from conventionalcommits.org.

Mouvedia avatar Mouvedia commented on May 26, 2024 1

@damianopetrungaro this list should be 'build', 'ci', 'chore', 'docs', 'perf', 'refactor', 'revert', 'style', 'test'.

cf https://github.com/marionebl/commitlint/blob/master/%40commitlint/config-conventional/index.js#L19

from conventionalcommits.org.

damianopetrungaro avatar damianopetrungaro commented on May 26, 2024 1

IMHO sth like that may work:

3dd7c21 revert: typo and html anchor links for pt-br (#159) [Benjamin E. Coe]
b5cfe31 revert(lang): add russian translation for v1.0.0-beta.3 [Damiano Petrungaro]
32d6831 revert(lang): typo in features word in russian docs [Damiano Petrungaro]
c1ffdb4 feat: Enable translation on the website [Damiano Petrungaro]
477028b feat(it): Add beta.4 [Damiano Petrungaro]
3bbc7de feat(it): Add beta.3 [Damiano Petrungaro]
6729804 fix: Add parse-commit-message to the specs [Damiano Petrungaro]
89f040c docs: add Git Commit Template plugin to the tool section (#164) [Benjamin E. Coe]
3daaa7e feat(lang): Add zh-TW translation for version v1.0.0-beta.4 (#163) [Benjamin E. Coe]
3dd7c2a fix: typo and html anchor links for pt-br (#159) [Benjamin E. Coe]
b5cfe3a feat(lang): add russian translation for v1.0.0-beta.3 [Damiano Petrungaro]
32d6830 fix(lang): typo in features word in russian docs [Damiano Petrungaro]
23a4585 fix(security): address security vulnerability in node-sass (#158) [GitHub]
4480927 feat(lang): add brazilian portuguese (pt-br) language (#157) [Benjamin E. Coe]
584fd57 feat: Add Korean translation [Damiano Petrungaro]
ca2f1f5 feat(lang): add Chinese translation for 1.0.0-beta.4 (#155) [Steve Mao]
e89f89f feat: add french translation for 1.0.0-beta.4 (#153) [Benjamin E. Coe]

from conventionalcommits.org.

Mouvedia avatar Mouvedia commented on May 26, 2024 1

Maybe instead of a requirement adding references could be a recommendation.

from conventionalcommits.org.

damianopetrungaro avatar damianopetrungaro commented on May 26, 2024

Thank you for opening the issue @Mouvedia .
Good one ;)

I would like to specify that we do not stick to the angular convention we are just inspired by it.

Anyway we miss the revert type.
IMHO is fine to use the Git default one Revert "feat: Add birthdate to user model" because it doesn't add any overhead during the reverting phase and is explicit because it wrap a conventional commit message.

Let's see what @conventional-commits/committee think about it.

from conventionalcommits.org.

Mouvedia avatar Mouvedia commented on May 26, 2024

@damianopetrungaro would the commit "feat(user): Add birthdate" be the revert of the commit "feat(user): Remove birthdate"?

Should the type always match the reverted commit's type? Should the message be identical?

from conventionalcommits.org.

damianopetrungaro avatar damianopetrungaro commented on May 26, 2024

@Mouvedia IMHO it makes sense to be Revert "feat: Add birthdate to user model" or revert: Add birthdate to user model because Remove birthday is not a feature itself, it may be part of a fix (but this means it's not reverted).

from conventionalcommits.org.

Mouvedia avatar Mouvedia commented on May 26, 2024

Personally I use the revert type for commits which stated purpose is to revert. For example if the original commit has some valuable artifacts, I end up with a partial revert.

e.g. revert: a6c8e75

from conventionalcommits.org.

damianopetrungaro avatar damianopetrungaro commented on May 26, 2024

Yup, I got it, I said my opinion but before adding it to the specs I want also other people from the committee

from conventionalcommits.org.

damianopetrungaro avatar damianopetrungaro commented on May 26, 2024

So @hbetts we have to face how to make it standardized also for processes that are not a simply git revert.

What do you think about it?

from conventionalcommits.org.

hutson avatar hutson commented on May 26, 2024

partial revert

Thank you @Mouvedia for the link. I was failing to see that you are reverting only part of a commit and not the entire commit.

So @hbetts we have to face how to make it standardized also for processes that are not a simply git revert.

So this may come down to semantics for me.

As an example, I have a commit that I introduced a long time ago. Turns out, that commit, which may have introduced a feature, also changed a line of code that, unintentionally, broke existing behavior.

I could do a partial revert. I could end up with a commit, of type revert, that removes the modification to that line of code.

However, isn't that also a fix?

from conventionalcommits.org.

damianopetrungaro avatar damianopetrungaro commented on May 26, 2024

yeah, you're right, this is a fix anyway.

However, isn't that also a fix?

from conventionalcommits.org.

Mouvedia avatar Mouvedia commented on May 26, 2024

Most of the time, it's the other way around: you revert the commit in the spirit on its initial message but leave out some artifacts that are worth keeping. Don't throw the baby out with the bathwater.

If it's not considered a revert, you won't use the revert type obviously.

from conventionalcommits.org.

damianopetrungaro avatar damianopetrungaro commented on May 26, 2024

But the point is that when you revert a feature (or a fix) you are creating a fix for it.

@hbetts what about other commit types like docs/chore?
They aren't types that should trigger a version update, but anyhow you are reverting a commit.

So they will be just docs(api): remove users from documented resources for example.

from conventionalcommits.org.

damianopetrungaro avatar damianopetrungaro commented on May 26, 2024

@Mouvedia but revert not being part of the specs we may just enforce the usage in the FAQ section.

from conventionalcommits.org.

damianopetrungaro avatar damianopetrungaro commented on May 26, 2024

@Mouvedia any other idea on this?

from conventionalcommits.org.

vmx avatar vmx commented on May 26, 2024

This discussion reads like there is an agreement that if you git revert a commit, the commit message should just be what Git is doing:

Revert "<the original header>"

This reverts commit <commit hash>.

<Put additional information here>

If it's a partial revert, you would use one of the existing types as it's probably a "fix", but it could e.g. also be "docs". You probably re-word the header anyway.

from conventionalcommits.org.

damianopetrungaro avatar damianopetrungaro commented on May 26, 2024

@stevemao @hutson @vmx any other feedback on this?

from conventionalcommits.org.

hutson avatar hutson commented on May 26, 2024

@hbetts what about other commit types like docs/chore?

If I introduce documentation that is inaccurate, I tend to revert the commit, use the docs type, along with a description that I'm correcting inaccurate information. Same thing as chore.

Again, this is just how I approach reverts. They may be some benefit to using, in-part, or in-whole, the git revert commit message that I am not aware of. Not to mention, others may find using revert commits to be useful in their own management of their projects, or useful in their tooling.

conventional-changelog currently implements revert exactly the same way as the angular docs. - @stevemao

That's a good point. We already follow the Angular convention. Are there sufficient benefits in changing?

from conventionalcommits.org.

stevemao avatar stevemao commented on May 26, 2024

Revert "<the original header>" is default so you don't need to change anything. The angular way you'd have to modify the header slightly: revert: <the original header>

from conventionalcommits.org.

damianopetrungaro avatar damianopetrungaro commented on May 26, 2024

We are inspired by the Angular convention, we are not following it.

That's a good point. We already follow the Angular convention. Are there sufficient benefits in changing?

And in this way we'll be consistnet with all the other commit types

That's a good point. We already follow the Angular convention. Are there sufficient benefits in changing?

from conventionalcommits.org.

hutson avatar hutson commented on May 26, 2024

We are inspired by the Angular convention, we are not following it.

😉

@damianopetrungaro do you feel we should be inspired in a different direction than the standard Angular convention? 😃

from conventionalcommits.org.

bcoe avatar bcoe commented on May 26, 2024

👋 my two cents,revert: currently fits within the scope of the existing specification, and it seems fair to put the responsibility for how this is interpreted in the hands of the upstream tooling authors. I'd advocate that we document it in the FAQ like @damianopetrungaro suggests.

from conventionalcommits.org.

Mouvedia avatar Mouvedia commented on May 26, 2024

I am fine with it but it needs to allow partial reverts as well.

from conventionalcommits.org.

bcoe avatar bcoe commented on May 26, 2024

@Mouvedia I think from conventionalcommits.org's point of view, it only cares about the prefix revert: , the tooling author would be able to decide the meaning of everything after the : , which might be a partial revert or full revert.

from conventionalcommits.org.

Mouvedia avatar Mouvedia commented on May 26, 2024

In light of TCR's emergence, the revert type may become as essential as fix and feat.

from conventionalcommits.org.

damianopetrungaro avatar damianopetrungaro commented on May 26, 2024

@bcoe @Mouvedia @hutson do we want to add it in the specs?

from conventionalcommits.org.

Mouvedia avatar Mouvedia commented on May 26, 2024

If it allows partial reverts and has the requirement to include the SHA(s) of the guilty commit(s) in the message, yes we should.

from conventionalcommits.org.

bcoe avatar bcoe commented on May 26, 2024

I would advocate that for revert commits we suggest in the footer "refs: x" which could be a sha or PR number. This seems less fragile than wrapping the original message.

It's then up to tooling on a specific platform to lookuo the ref.

from conventionalcommits.org.

bcoe avatar bcoe commented on May 26, 2024

see: https://github.com/conventional-commits/conventionalcommits.org/pull/175/files#diff-4d1a596d11b78953177b1c0bc67d9f4fR176

I've added the following blurb regarding revert commits:

### How does Conventional Commits handle revert commits?

Reverting code can be complicated: are you reverting multiple commits? if you revert a feature, should the next release
instead be a patch?

Conventional Commits does not make an explicit effort to define revert behavior. Instead we leave it to tooling
authors to use the flexility of _types_ and _footers_ to develop their logic for handling reverts.

One recommendation is to use the `revert` type, and a footer that references the commit SHAs that are being reverted:

I'm convinced that it's beyond the scope of the Conventional Commit specification to strictly define how reverts should be managed, but I think this FAQ provides a reasonable starting point for tooling authors.

I personally like a revert: type with Refs: footer because:

  • unlike: Revert "<the original header>", revert: follows the existing specification (I don't like the idea of extending the spec for this edge-case).
  • it's conceivable that you want to convert a collection of commits, and it seems like using a footer gives more flexibility.

from conventionalcommits.org.

Mouvedia avatar Mouvedia commented on May 26, 2024

Refs: #676104e, #a215868

You should remove the #.

from conventionalcommits.org.

soullivaneuh avatar soullivaneuh commented on May 26, 2024

What is the state of this issue?

Personally, I just used the git revert command, leaving the original message and the release process works well:

image

from conventionalcommits.org.

nm-remarkable avatar nm-remarkable commented on May 26, 2024

I recommend.

Reverts should still follow the other standards, therefore a revert commit should be revert: "original revert message" and also that the following additions should be made to it.

    - the body of the commit should contain an explanation of WHY the commit was reverted
    - the body of the commit should have the hash of the offending commit (or multiple)
    - the header of the commit should be amended to `revert: "OG message"`

from conventionalcommits.org.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.