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

Ignore version bump for non-conventional commits #22

Open
ivandov opened this issue Jun 23, 2021 · 16 comments
Open

Ignore version bump for non-conventional commits #22

ivandov opened this issue Jun 23, 2021 · 16 comments

Comments

@ivandov
Copy link

ivandov commented Jun 23, 2021

In most conventions supported with conventional-changelog, there are commit types that should not trigger a version bump.

For example, when using the angular or conventionalcommits preset, version bumps should only be applicable if using commits that begin with fix, feat, or other types that include a ! to signify a BREAKING CHANGE.

For commit messages that either do not conform to the types for the specified preset or are not one of the types listed above that signify a version change, release-it and this plugin should ignore the creation of a new semver release.

Is this achievable by this plug-in, since it overrides the standard version bump logic that happens in release-it?

@webpro
Copy link
Contributor

webpro commented Jul 14, 2021

The options provided to this plugin are sent to its underlying dependency: https://github.com/conventional-changelog/conventional-changelog/tree/master/packages/conventional-recommended-bump#readme

However, at first glance I don't see an option for what you are trying to achieve. But maybe it's standard behavior?

@ivandov
Copy link
Author

ivandov commented Jul 16, 2021

Right, my experience here was from using semantic-release and how it would ignore version bumps unless commit messages actually matched the criteria outlined in the convention specified.

@webpro
Copy link
Contributor

webpro commented Aug 25, 2021

Did a little bit of digging and testing, and I can see the conventional-recommended-bump dependency for this plugin gives only major, minor and patch. Never null or undefined, regardless of the commit messages:

{
  level: 2,
  reason: 'There are 0 BREAKING CHANGES and 0 features',
  releaseType: 'patch'
}

Only when a whatBump function is provided to parse the commits manually. However, that would defeat the whole purpose of this plugin, imho.

@ivandov
Copy link
Author

ivandov commented Aug 26, 2021

As mentioned in release-it/release-it#798 by vonovak... This sounds like something that should be an optional flag for modifying the return types of conventional-recommended-bump.

I think that'd be a valid contribution if something along these lines doesn't already exist there. And, I'd also question how semantic-release is achieving its semver determination? Is there just a different library that can be used?

@webpro
Copy link
Contributor

webpro commented Aug 26, 2021

So there are a few options:

  • Add a whatBump() function to the options for this dependency, and parse the commits in this plugin (to add a return value of null or undefined).
  • Move the getIncrementedVersion() method to a new plugin (the changelog generation could stay here).
  • Add a hook with a CLI command to detect whether there's a bump upfront (e.g. in the before:init hook), and cancel the release if negative.

@ivandov
Copy link
Author

ivandov commented Aug 26, 2021

I'm not sure any of these options provide a really good solution...

Implementing a whatBump() function that would properly handle all the conventions that this plugin support and conditionally ignoring a version bump if it doesn't meet some standard... would essentially be re-implementing a core piece of what conventional-recommended-bump likely already does internally.

To echo your sentiment:

Only when a whatBump function is provided to parse the commits manually. However, that would defeat the whole purpose of this plugin, imho.

Moving the getIncrementedVersion() method to another plugin seems like a misplaced solution that causes tight dependencies between plugins, which I imagine is not intended.

Adding a hook to the CLI command puts all the onus on the user of this plugin and is subject to significant user error.


Personally, I think this type of logic should really be a core feature in the conventional-recommended-bump library. I also looked through its interfaces and did not see any published mechanism for what would be needed here.

Haven't had a chance to look, but, I'd be curious what library semantic-release uses to handle this exact scenario internally... Did they code up their own?

@webpro
Copy link
Contributor

webpro commented Aug 27, 2021

Totally agreed.

Looks like semantic-release has a analyzeCommits step/plugin which parses commits itself (and potentially return null): https://github.com/semantic-release/commit-analyzer/blob/master/index.js

@vonovak
Copy link

vonovak commented Apr 30, 2022

Hello @webpro, I'm wondering, has this been fixed, or do you plan to fix it?
I'd love to run release-it in CI but if it makes releases for commits that should not trigger one, it's not possible.
Thank you 🙂

@webpro
Copy link
Contributor

webpro commented Apr 30, 2022

Looking into it again, I'm seeing basically all presets for recommended bump work the same. Here's the angular preset: https://github.com/conventional-changelog/conventional-changelog/blob/master/packages/conventional-changelog-angular/conventional-recommended-bump.js (this is the whatBump function).

It always returns value 0, 1 or 2 for level, never null or undefined, always resulting in a value of this array here: https://github.com/conventional-changelog/conventional-changelog/blob/master/packages/conventional-recommended-bump/index.js#L10

However, I just released a small update of release-it (warning: it's only in the upcoming release-it v15), which together with a custom whatBump option could do what you want. I've added a spec in this repo (warning: only in v5): 124351c

So, with release-it v15 and this plugin v5 this would allow something like this in .release-it.js:

module.exports = {
  plugins: {
    '@release-it/conventional-changelog': {
      whatBump: commits => ({ level: null, reason: 'Parsed commits do not warrant a version bump.' })
    }
  }
};

Still, you would need to properly implement the whatBump function.

(You could use this already using the esm npm dist tags. Planning to release the majors shortly, though.)

@ivandov
Copy link
Author

ivandov commented May 2, 2022

@webpro How does your example code snippet leverage whatBump to properly decide when to send null for the level?

Are you suggesting that the function would be extended with additional conditional logic to properly determine when to set the null value?

@webpro
Copy link
Contributor

webpro commented May 2, 2022

@webpro How does your example code snippet leverage whatBump to properly decide when to send null for the level?

Are you suggesting that the function would be extended with additional conditional logic to properly determine when to set the null value?

My code example is only to indicate where the function fits in the release-it/plugin config.

So yes, additional logic is required, much like the original (linked to above).

@jamesvillarrubia
Copy link

jamesvillarrubia commented Mar 1, 2023

Following up on this. I have created a whatBump that seems to align more with the intent of the angular preset and the semantic-release version.

Here's the function:

whatBump: (commits,options)=>{
          let defaults = {
            build: 'ignore',
            ci: 'ignore',
            docs: 'ignore',
            feat: 'minor',
            fix: 'patch',
            perf: 'patch',
            refactor: 'ignore',
            test: 'ignore'
          }
          let types = (options?.preset?.types || [])
          .reduce((a, v) => {
            return { ...a, [v.type]: v.release}
          }, {}) 

          types = Object.assign({},defaults,types)
          let breakings = 0
          let features = 0
          let levelSet = ['major','minor','patch','ignore']
          let level = Math.min.apply(Math, commits.map(commit => {
            let level = levelSet.indexOf(types[commit.type])
            level = level<0?3:level
            if (commit.notes.length > 0) {
              breakings += commit.notes.length
            }
            if(commit.type === 'feat'){
              features += 1;
            }
            return level
          }))
      
          return {
            level: level,
            reason: breakings === 1
              ? `There is ${breakings} BREAKING CHANGE and ${features} features`
              : `There are ${breakings} BREAKING CHANGES and ${features} features`
          }
}

You can then combine this with:

"preset": {
        "name": "conventionalcommits",
        "types": [
          {
            "type": "refactor",
            "release": "patch"
          },
          {
            "type": "style",
            "release": "patch"
          },
          {
            "type": "perf",
            "release": "patch"
          },
          {
            "type": "chore",
            "release": "patch"
          },
          {
            "type": "ci",
            "release": "patch"
          }
        ]
      }

This gives you the ability to override the specific types with one of ['major', 'minor', 'patch', 'ignore']. You can also set your own types.

@jamesvillarrubia
Copy link

I'm happy to make a PR if it's deemed appropriate. Possibly to the angular plugin or the conventionalcommit. @webpro

@webpro
Copy link
Contributor

webpro commented Jul 29, 2023

That would be nice, @jamesvillarrubia! Are you still up for this? Would be great to have options to ignore certain types of commits.

@mogelbrod
Copy link

Following up on this. I have created a whatBump that seems to align more with the intent of the angular preset and the semantic-release version.

Thanks for sharing! I might be wrong, but it doesn't look like the code does a major bump on ...!: ... and BREAKING CHANGES style commits?

@webpro
Copy link
Contributor

webpro commented Nov 12, 2023

Maybe the function can be extended; I'll be happy to add it to the docs or maybe even merge it into the codebase (with some logic/option to enable it).

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

No branches or pull requests

5 participants