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

[determineNextVersion] lte fails when currentVersion was sourced from first Git commit #1261

Closed
10hendersonm opened this issue May 27, 2020 · 2 comments · Fixed by #1262
Closed
Labels
released This issue/pull request has been released.

Comments

@10hendersonm
Copy link
Contributor

The npm plugin is calling getLastTagNotInBaseBranch, however that code falls off to the first Git commit if it can't find a tag. This value is passed as currentVersion to getLastTagNotInBaseBranch, which is throwing when it attempts to call lte on the commit hash.

I believe I managed to fix it locally by wrapping an extra parse check in, however I'm having some trouble understanding the context of the code so I'm not sure that's the right approach:

+ return !next || !semver.parse(currentVersion) || lte(next, currentVersion)
- return !next || lte(next, currentVersion) 

return !next || lte(next, currentVersion)

@hipstersmoothie
Copy link
Collaborator

I recently made this change but didn't account for this. I think the code I added here should not have returned the first commit since it isn't a tag. Undefined makes more sense, [https://github.com/intuit/auto/blob/9a3c6561453b881773316974f6adb0ed0df347d3/packages/core/src/auto.ts#L1247] this is where it should have fallen back to the first commit instead.

I'm pretty sure that won't fix the problem though. the determineNextVersion should accept string | undefined for last and currentVersion and default to 0.0.0 if not present.

Would you like to try to get this in as a PR? If not I can probably look at it in the next day or two

@adierkens
Copy link
Collaborator

🚀 Issue 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
released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants