go-conventionalcommits

go-conventionalcommits copied to clipboard

Adds methods to determine if a given commit is a fix or feature commit and one method to determine which version bump a given commit mandates

Thanks for your review and the valuable input. I'd also prefer the solution which stores the type set configuration in the output message. I'll adapt my MR accordingly.

And to sort some things out:

• the conventional commit specification is not explicit about bumping anything else than patch, feat or breaking changes. Therefore I implemented other types as an "unknown" bump for now. Although many other tools in the field do bump the patch version on any type other than feat or breaking change. Which way do you prefer for this tool?
• in the CONTRIBUTING.md of falco (https://github.com/falcosecurity/.github/blob/master/CONTRIBUTING.md#commit-convention) the new type (as used in your example) is not mentioned. I'm not familiar with falco, but probably you have some more experience with it and can shed some light on this?

* the conventional commit specification is not explicit about bumping anything else than `patch`, `feat` or `breaking changes`. Therefore I implemented other types as an "unknown" bump for now. Although many other tools in the field do bump the patch version on any type other than `feat` or `breaking change`. Which way do you prefer for this tool?

Agree, let's stick with only what is mandated by the spec.

* in the `CONTRIBUTING.md` of falco (https://github.com/falcosecurity/.github/blob/master/CONTRIBUTING.md#commit-convention) the `new` type (as used in your example) is not mentioned. I'm not familiar with falco, but probably you have some more experience with it and can shed some light on this?

You' right: the "falco" types are not standardized yet. Being one of its authors, I can tell you that new: is used to communicate new features in Falco. It's basically a synonym of feat: IMHO.

OK, thanks a lot for the clarification.

So, if I got it right the current ruleset should look something like this:

• patch bump if type == "patch", regardless of TypeConfig
• minor bump if type == "feat", regardless of TypeConfig
• minor bump if type == "new", if TypeConfig == TypesFalco
• major bump if exclamation mark or breaking-changes footer is present, regardless of TypeConfig

Altough with this ruleset there's some potential for inconsistency:

• TypesConventional bumping "to the letter" (see discussion above about the "unknown" bump https://github.com/leodido/go-conventionalcommits/pull/17#issuecomment-981602231)
• TypesFalco bumping "to the letter"
• TypesMinimal there's no literal description about bumping for this type, so we stick implicitly to the conventional commit spec - or we try to be consistent across TypeConfigs and return unkown bump for all commits (since nothing/no one mandates the bumps)
• TypesFreeForm see TypesMinimal

Do you understand my point? And what do you think about it?

Hello @juckerf

I almost forgot this, sorry!

For clarity, let's split the two problems we have here.

The first problem is "how to keep track of which types' set the parser is using".

I think we agree on implementing the solution (2) that I proposed above: keep track of which types' set was used with an index.

The second problem is "how to bump the version depending on the conventional types (and the types' set they belong to)"

I also see your point about inconsistencies. I believe there will always be inconsistencies in this case, especially for types' set different from the TypesConventional one.

So, I think the correct way to do this would be to think of the version bump as a strategy, that depends on two things:

1. which types' set the ConventionalCommit instance was created from
2. what exact type prefix (.Type field) it contains

I envision that the func (c *ConventionalCommit) VersionBump() VersionBump {...} function you wrote should take a callback that actually implements the version bumping strategy.

Let's say we have the following declaration:

type VersionBumpStrategy func(*ConventionalCommit) VersionBump

Then, we'd have:

func (c *ConventionalCommit) VersionBump(strategy VersionBumpStrategy) VersionBump {
if strategy == nil {
return defaultStrategy(c)
}

return strategy(c)
}

This way, we can provide a default opinionated (by us) strategy, which can also contain inconsistencies.

We could even go further and provide different default strategies for the different types' sets we have... Making this mechanism able to support future types' set we may introduce without hassles.

But, more than everything, we provide to the users of this library a way to specify their strategy for bumping the versions.

WDYT? :)

Sorry, I had to find some time to have a look at this. I like the strategy-approach which gives a lot of flexibility for users and implemented it right away.

@leodido do you have time to have a look at this and perhaps merge this PR if everything is fine with you?

Welcome @juckerf! Thank you so much for your first pull request!

@leodido any update on this?

Sorry for leaving this behind (I've been way too busy)...

Taking a look at it rn, thank you!

Thank you @juckerf for this first contribution!

Reviewpad Report

:warning: Warnings

• Please link an issue to the pull request

Sorry for leaving this behind (I've been way too busy)...

Taking a look at it rn, thank you!

No worries, I can relate :-) Thanks for the merge!