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

feat: allows seperate prefixTag version sequences #573

Merged
merged 7 commits into from
Apr 6, 2021

Conversation

jahead
Copy link
Contributor

@jahead jahead commented Apr 17, 2020

Solves #572

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0005%) to 99.429% when pulling c8016dd on jahead:master into 2f04ac8 on conventional-changelog:master.

@jvitor83
Copy link

jvitor83 commented Feb 27, 2021

@bcoe need that PR merged and a new release please.
Solves #702 too.

@jahead
Copy link
Contributor Author

jahead commented Feb 28, 2021

there's conflicts I'll fix them on the weekend and update

@jvitor83
Copy link

@jahead don't forget that please. :)

@codecov-io
Copy link

codecov-io commented Apr 4, 2021

Codecov Report

Merging #573 (1c93d7f) into master (f5bff12) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #573   +/-   ##
=======================================
  Coverage   97.71%   97.71%           
=======================================
  Files          24       24           
  Lines        1051     1052    +1     
=======================================
+ Hits         1027     1028    +1     
  Misses         24       24           
Impacted Files Coverage Δ
index.js 100.00% <100.00%> (ø)
lib/latest-semver-tag.js 100.00% <100.00%> (ø)
test/git.spec.js 99.07% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5bff12...1c93d7f. Read the comment docs.

@jahead
Copy link
Contributor Author

jahead commented Apr 4, 2021

Done. Sorry for taking so long. Let me know if anything else is needed

@@ -66,7 +66,7 @@ function mock ({ bump, changelog, tags }) {
}
}))

mockery.registerMock('git-semver-tags', function (cb) {
mockery.registerMock('git-semver-tags', function (_, cb) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jahead this is looking good to me, but any chance we could add a test that covers the additional functionality?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea mate, I"ll do it tonight.

I was thinking how to test this

Copy link
Contributor Author

@jahead jahead Apr 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bcoe Hey so I added some tests here and tested tagPrefix in general, as there didn't seem to be any tests already that I could see.

I'm not overly happy with is this as git-semver-tags actually is the one filtering the tags, and due to the mock we won't know if git-semver-tags changes will break the logic here. But the tests cover the use case in #702

resolve if you're happy

@jahead
Copy link
Contributor Author

jahead commented Apr 6, 2021

Oddly, through testing this.
I discovered/re-discovered that

    if (pkg) {
      version = pkg.version
    } else if (args.gitTagFallback) {
      version = await latestSemverTag(args.tagPrefix)
    } else {
      throw new Error('no package file found')
    }

Which means that if there is a packageFiles defined we never get to use the gitSemverTags logic to resolve versions of different prefixes. And there's no option to force resolution by tag if packageFiles are defined.

It will solve #702 but people should take care to configure no packageFiles.

I'm not sure we should solve that problem in this PR, but it did seem like odd to me.
I've outlined it in the tests.

@bcoe bcoe merged commit 3bbba02 into conventional-changelog:master Apr 6, 2021
@bcoe
Copy link
Member

bcoe commented Apr 6, 2021

@jahead released in v9.2.0, and appreciated. Let me know if there are any issues.

I do gradually try to make my way through @ messages on GitHub, so feel free to keep engaging in this way.

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

Successfully merging this pull request may close these issues.

5 participants