intuit / auto

Generate releases based on semantic version labels on pull requests.
https://intuit.github.io/auto/
MIT License
2.28k stars 205 forks source link

Fix rare issue with canary id from git sha #2448

Closed MichaelRyanWebber closed 8 months ago

MichaelRyanWebber commented 8 months ago

In the rare case that the first 7 characters of a git sha contains only numbers and begins with 0, SemVer will reject the canary (due to this part of the canaryIdentifier) as an invalid version.

In such cases, make sure we include enough of the sha to get a letter.

Ignore the ~1:1.5billion chance that SHA is all numbers starting with 0, but optionally the sha could be replaced with the string jackpot

Todo:

Change Type

Indicate the type of change your pull request is:

MichaelRyanWebber commented 8 months ago

@hipstersmoothie Not sure how you'd actually want to solve this case, but figured I'd throw an attempt at it

could also do lte(next, currentVersion, true) here to get the "loose" checking, but I'd guess there would be issues later since "full" is the default.

the two regexes for reference:

full /^v?(0|[1-9]\d{0,256})\.(0|[1-9]\d{0,256})\.(0|[1-9]\d{0,256})(?:-((?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250})(?:\.(?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250}))*))?(?:\+([a-zA-Z0-9-]{1,250}(?:\.[a-zA-Z0-9-]{1,250})*))?$/ loose /^[v=\s]*(\d{1,256})\.(\d{1,256})\.(\d{1,256})(?:-?((?:\d{1,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250})(?:\.(?:\d{1,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250}))*))?(?:\+([a-zA-Z0-9-]{1,250}(?:\.[a-zA-Z0-9-]{1,250})*))?$/

hipstersmoothie commented 8 months ago

@MichaelRyanWebber this looks great! such an odd issues to hit. my only note is a comment here would be nice

codecov[bot] commented 8 months ago

Codecov Report

Attention: Patch coverage is 75.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 80.55%. Comparing base (b8d1af6) to head (c214a53). Report is 4 commits behind head on main.

Files Patch % Lines
packages/core/src/auto.ts 75.00% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2448 +/- ## ========================================== - Coverage 80.63% 80.55% -0.08% ========================================== Files 69 69 Lines 5680 5683 +3 Branches 1335 1336 +1 ========================================== - Hits 4580 4578 -2 - Misses 718 719 +1 - Partials 382 386 +4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

hipstersmoothie commented 8 months ago

Once you add the comment I'll merge

MichaelRyanWebber commented 8 months ago

@hipstersmoothie added a comment. lmk if that seems clear enough!

hipstersmoothie commented 8 months ago

It's out 🎉

laughedelic commented 8 months ago

Hi @hipstersmoothie, release of this changes doesn't have the binaries attached as assets: https://github.com/intuit/auto/releases/tag/v11.1.3 Can you fix it or release another version?