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

fix(git): Prevents getLastTagNotInBaseBranch from returning a commit hash #1262

Conversation

10hendersonm
Copy link
Contributor

@10hendersonm 10hendersonm commented May 28, 2020

closes #1261

What Changed

This PR removes functionality where getLastTagNotInBaseBranch would previously return the first commit on a branch if no tag was found.

Why

getLastTagNotInBaseBranch is called in the npm plugin, but the value is then passed to determineNextVersion, where it attempts to run semver operations on the provided tag. When the tag is instead a commit hash, the semver operations fail out gracelessly.

Within #1261, @hipstersmoothie suggests making determineNextVersion take optional lastVersion? and currentVersion?s with appropriate handling, however I'm unsure that's necessary given all calls or references to getLastTagNotInBaseBranch are currently piped over to some default value (1, 2, 3 - managed by this PR, 4).

Unfortunately, I'm having difficulty further validating, as my monorepo no longer falls into this state..

Todo:

  • Add tests
  • Add docs
📦 Published PR as canary version: under canary scope @auto-canary@9.36.3-canary.1262.16149.0

✨ Test out this PR locally via:

npm install @auto-canary/bot-list@9.36.3-canary.1262.16149.0
npm install @auto-canary/auto@9.36.3-canary.1262.16149.0
npm install @auto-canary/core@9.36.3-canary.1262.16149.0
npm install @auto-canary/all-contributors@9.36.3-canary.1262.16149.0
npm install @auto-canary/brew@9.36.3-canary.1262.16149.0
npm install @auto-canary/chrome@9.36.3-canary.1262.16149.0
npm install @auto-canary/cocoapods@9.36.3-canary.1262.16149.0
npm install @auto-canary/conventional-commits@9.36.3-canary.1262.16149.0
npm install @auto-canary/crates@9.36.3-canary.1262.16149.0
npm install @auto-canary/exec@9.36.3-canary.1262.16149.0
npm install @auto-canary/first-time-contributor@9.36.3-canary.1262.16149.0
npm install @auto-canary/gem@9.36.3-canary.1262.16149.0
npm install @auto-canary/gh-pages@9.36.3-canary.1262.16149.0
npm install @auto-canary/git-tag@9.36.3-canary.1262.16149.0
npm install @auto-canary/gradle@9.36.3-canary.1262.16149.0
npm install @auto-canary/jira@9.36.3-canary.1262.16149.0
npm install @auto-canary/maven@9.36.3-canary.1262.16149.0
npm install @auto-canary/npm@9.36.3-canary.1262.16149.0
npm install @auto-canary/omit-commits@9.36.3-canary.1262.16149.0
npm install @auto-canary/omit-release-notes@9.36.3-canary.1262.16149.0
npm install @auto-canary/released@9.36.3-canary.1262.16149.0
npm install @auto-canary/s3@9.36.3-canary.1262.16149.0
npm install @auto-canary/slack@9.36.3-canary.1262.16149.0
npm install @auto-canary/twitter@9.36.3-canary.1262.16149.0
npm install @auto-canary/upload-assets@9.36.3-canary.1262.16149.0
# or 
yarn add @auto-canary/bot-list@9.36.3-canary.1262.16149.0
yarn add @auto-canary/auto@9.36.3-canary.1262.16149.0
yarn add @auto-canary/core@9.36.3-canary.1262.16149.0
yarn add @auto-canary/all-contributors@9.36.3-canary.1262.16149.0
yarn add @auto-canary/brew@9.36.3-canary.1262.16149.0
yarn add @auto-canary/chrome@9.36.3-canary.1262.16149.0
yarn add @auto-canary/cocoapods@9.36.3-canary.1262.16149.0
yarn add @auto-canary/conventional-commits@9.36.3-canary.1262.16149.0
yarn add @auto-canary/crates@9.36.3-canary.1262.16149.0
yarn add @auto-canary/exec@9.36.3-canary.1262.16149.0
yarn add @auto-canary/first-time-contributor@9.36.3-canary.1262.16149.0
yarn add @auto-canary/gem@9.36.3-canary.1262.16149.0
yarn add @auto-canary/gh-pages@9.36.3-canary.1262.16149.0
yarn add @auto-canary/git-tag@9.36.3-canary.1262.16149.0
yarn add @auto-canary/gradle@9.36.3-canary.1262.16149.0
yarn add @auto-canary/jira@9.36.3-canary.1262.16149.0
yarn add @auto-canary/maven@9.36.3-canary.1262.16149.0
yarn add @auto-canary/npm@9.36.3-canary.1262.16149.0
yarn add @auto-canary/omit-commits@9.36.3-canary.1262.16149.0
yarn add @auto-canary/omit-release-notes@9.36.3-canary.1262.16149.0
yarn add @auto-canary/released@9.36.3-canary.1262.16149.0
yarn add @auto-canary/s3@9.36.3-canary.1262.16149.0
yarn add @auto-canary/slack@9.36.3-canary.1262.16149.0
yarn add @auto-canary/twitter@9.36.3-canary.1262.16149.0
yarn add @auto-canary/upload-assets@9.36.3-canary.1262.16149.0

@codecov
Copy link

codecov bot commented May 28, 2020

Codecov Report

Merging #1262 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1262   +/-   ##
=======================================
  Coverage   80.98%   80.98%           
=======================================
  Files          56       56           
  Lines        4075     4076    +1     
  Branches      879      880    +1     
=======================================
+ Hits         3300     3301    +1     
  Misses        552      552           
  Partials      223      223           
Impacted Files Coverage Δ
packages/core/src/auto.ts 76.88% <100.00%> (ø)
packages/core/src/git.ts 88.72% <100.00%> (+0.03%) ⬆️

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 9a3c656...3f87c8a. Read the comment docs.

@10hendersonm 10hendersonm marked this pull request as ready for review May 28, 2020 16:26
@hipstersmoothie hipstersmoothie added the patch Increment the patch version when merged label May 28, 2020
@hipstersmoothie
Copy link
Collaborator

I'll test this out later today! Thanks for the contribution

@hipstersmoothie hipstersmoothie merged commit fd3e10d into intuit:master May 28, 2020
@adierkens
Copy link
Collaborator

🚀 PR was released in v9.36.4 🚀

@adierkens adierkens added the released This issue/pull request has been released. label May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[determineNextVersion] lte fails when currentVersion was sourced from first Git commit
3 participants