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 7 commits into from Apr 6, 2021


Copy link

@jahead jahead commented Apr 17, 2020

Solves #572

Copy link

@coveralls coveralls commented Apr 17, 2020

Coverage Status

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

Copy link

@jvitor83 jvitor83 commented Feb 27, 2021

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

Copy link
Contributor Author

@jahead jahead commented Feb 28, 2021

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

Copy link

@jvitor83 jvitor83 commented Mar 29, 2021

@jahead don't forget that please. :)

Copy link

@codecov-io 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.

Copy link
Contributor Author

@jahead 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

@bcoe bcoe Apr 6, 2021

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

Copy link
Contributor Author

@jahead jahead Apr 6, 2021

Yea mate, I"ll do it tonight.

I was thinking how to test this

Copy link
Contributor Author

@jahead jahead Apr 6, 2021

@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

Copy link
Contributor Author

@jahead 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 approved these changes Apr 6, 2021
@bcoe bcoe merged commit 3bbba02 into conventional-changelog:master Apr 6, 2021
6 checks passed
Copy link

@bcoe 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
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants