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

Jump to bottom

fix(npm): Prevents canary releases with double dashed version numbers #1997

Merged
merged 1 commit into from May 25, 2021
Merged

fix(npm): Prevents canary releases with double dashed version numbers #1997

merged 1 commit into from May 25, 2021

Conversation

10hendersonm
Copy link
Contributor

@10hendersonm 10hendersonm commented May 24, 2021 •
edited by adierkens

What Changed

Removed leading dash on all npm canary releases.

Why

Since upgrading to auto@10.x, our canary releases are creating versions like 10.1.0--canary.1379.2451.0 with a double-dash on the pre-id.

The other instances in the canary hook were already stripping off the leading -, so I've extended this to the last 2 instances.

Todo:

  • Add tests
  • Add docs

Change Type

Indicate the type of change your pull request is:

  • documentation
  • patch
  • minor
  • major

🐤 Download canary assets:

auto-linux--canary.1997.24321.gz
auto-macos--canary.1997.24321.gz
auto-win.exe--canary.1997.24321.gz

📦 Published PR as canary version: under canary scope @auto-canary@10.29.1-canary.1997.24321.0

✨ Test out this PR locally via:

npm install @auto-canary/bot-list@10.29.1-canary.1997.24321.0
npm install @auto-canary/auto@10.29.1-canary.1997.24321.0
npm install @auto-canary/core@10.29.1-canary.1997.24321.0
npm install @auto-canary/package-json-utils@10.29.1-canary.1997.24321.0
npm install @auto-canary/all-contributors@10.29.1-canary.1997.24321.0
npm install @auto-canary/brew@10.29.1-canary.1997.24321.0
npm install @auto-canary/chrome@10.29.1-canary.1997.24321.0
npm install @auto-canary/cocoapods@10.29.1-canary.1997.24321.0
npm install @auto-canary/conventional-commits@10.29.1-canary.1997.24321.0
npm install @auto-canary/crates@10.29.1-canary.1997.24321.0
npm install @auto-canary/docker@10.29.1-canary.1997.24321.0
npm install @auto-canary/exec@10.29.1-canary.1997.24321.0
npm install @auto-canary/first-time-contributor@10.29.1-canary.1997.24321.0
npm install @auto-canary/gem@10.29.1-canary.1997.24321.0
npm install @auto-canary/gh-pages@10.29.1-canary.1997.24321.0
npm install @auto-canary/git-tag@10.29.1-canary.1997.24321.0
npm install @auto-canary/gradle@10.29.1-canary.1997.24321.0
npm install @auto-canary/jira@10.29.1-canary.1997.24321.0
npm install @auto-canary/magic-zero@10.29.1-canary.1997.24321.0
npm install @auto-canary/maven@10.29.1-canary.1997.24321.0
npm install @auto-canary/microsoft-teams@10.29.1-canary.1997.24321.0
npm install @auto-canary/npm@10.29.1-canary.1997.24321.0
npm install @auto-canary/omit-commits@10.29.1-canary.1997.24321.0
npm install @auto-canary/omit-release-notes@10.29.1-canary.1997.24321.0
npm install @auto-canary/pr-body-labels@10.29.1-canary.1997.24321.0
npm install @auto-canary/released@10.29.1-canary.1997.24321.0
npm install @auto-canary/s3@10.29.1-canary.1997.24321.0
npm install @auto-canary/sbt@10.29.1-canary.1997.24321.0
npm install @auto-canary/slack@10.29.1-canary.1997.24321.0
npm install @auto-canary/twitter@10.29.1-canary.1997.24321.0
npm install @auto-canary/upload-assets@10.29.1-canary.1997.24321.0
npm install @auto-canary/vscode@10.29.1-canary.1997.24321.0
# or 
yarn add @auto-canary/bot-list@10.29.1-canary.1997.24321.0
yarn add @auto-canary/auto@10.29.1-canary.1997.24321.0
yarn add @auto-canary/core@10.29.1-canary.1997.24321.0
yarn add @auto-canary/package-json-utils@10.29.1-canary.1997.24321.0
yarn add @auto-canary/all-contributors@10.29.1-canary.1997.24321.0
yarn add @auto-canary/brew@10.29.1-canary.1997.24321.0
yarn add @auto-canary/chrome@10.29.1-canary.1997.24321.0
yarn add @auto-canary/cocoapods@10.29.1-canary.1997.24321.0
yarn add @auto-canary/conventional-commits@10.29.1-canary.1997.24321.0
yarn add @auto-canary/crates@10.29.1-canary.1997.24321.0
yarn add @auto-canary/docker@10.29.1-canary.1997.24321.0
yarn add @auto-canary/exec@10.29.1-canary.1997.24321.0
yarn add @auto-canary/first-time-contributor@10.29.1-canary.1997.24321.0
yarn add @auto-canary/gem@10.29.1-canary.1997.24321.0
yarn add @auto-canary/gh-pages@10.29.1-canary.1997.24321.0
yarn add @auto-canary/git-tag@10.29.1-canary.1997.24321.0
yarn add @auto-canary/gradle@10.29.1-canary.1997.24321.0
yarn add @auto-canary/jira@10.29.1-canary.1997.24321.0
yarn add @auto-canary/magic-zero@10.29.1-canary.1997.24321.0
yarn add @auto-canary/maven@10.29.1-canary.1997.24321.0
yarn add @auto-canary/microsoft-teams@10.29.1-canary.1997.24321.0
yarn add @auto-canary/npm@10.29.1-canary.1997.24321.0
yarn add @auto-canary/omit-commits@10.29.1-canary.1997.24321.0
yarn add @auto-canary/omit-release-notes@10.29.1-canary.1997.24321.0
yarn add @auto-canary/pr-body-labels@10.29.1-canary.1997.24321.0
yarn add @auto-canary/released@10.29.1-canary.1997.24321.0
yarn add @auto-canary/s3@10.29.1-canary.1997.24321.0
yarn add @auto-canary/sbt@10.29.1-canary.1997.24321.0
yarn add @auto-canary/slack@10.29.1-canary.1997.24321.0
yarn add @auto-canary/twitter@10.29.1-canary.1997.24321.0
yarn add @auto-canary/upload-assets@10.29.1-canary.1997.24321.0
yarn add @auto-canary/vscode@10.29.1-canary.1997.24321.0

@adierkens adierkens added the patch Increment the patch version when merged label May 24, 2021
@codecov
Copy link

codecov bot commented May 24, 2021 •
edited

Codecov Report

Merging #1997 (f1ffa23) into main (5fa091a) will increase coverage by 0.07%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1997      +/-   ##
==========================================
+ Coverage   80.16%   80.24%   +0.07%     
==========================================
  Files          66       66              
  Lines        5375     5395      +20     
  Branches     1248     1249       +1     
==========================================
+ Hits         4309     4329      +20     
+ Misses        707      706       -1     
- Partials      359      360       +1
Impacted Files Coverage Δ
plugins/cocoapods/src/index.ts 88.95% <100.00%> (+1.45%) ⬆️
plugins/npm/src/index.ts 73.03% <100.00%> (+0.04%) ⬆️

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 1f27b91...f1ffa23. Read the comment docs.

@10hendersonm
Copy link
Contributor Author

Seeing this in my PR body was pretty funny:

image

I get the feeling my PR doesn't resolve that, as I've only touched npm.

@10hendersonm
Copy link
Contributor Author

I'm also up in the air about whether this is a patch or a major. If somebody has come to rely on regexes to parse semvers, it's breaking for them, but I also believe it's unintended in the current implementation, meaning it's a fix whether or not it breaks somebody having taken steps to consume it in the broken fashion.

It also seems super minor to rev up to v11 for just this.

TL;DR IMO it's a patch

@hipstersmoothie hipstersmoothie merged commit 1e83684 into intuit:main May 25, 2021
@rconnamacher
Copy link

rconnamacher commented May 26, 2021 •
edited

@hipstersmoothie @10hendersonm The double-hyphen was an intentional change made by @hipstersmoothie to cause canary releases to not override beta releases in standard Semver ranges (which considers prerelease tags in alphabetical order). A pull request opened by any non-contributor should never match a prerelase version range like >= 1.0.0-beta.1. Is that being undone?

@rconnamacher rconnamacher mentioned this pull request Jun 2, 2021
Closed
10hendersonm pushed a commit to 10hendersonm/auto that referenced this pull request Jun 7, 2021
Revert "Merge pull request intuit#1997 from 10hendersonm/canary_doubl…
85e8ecd 
…e_dash"

This reverts commit 1e83684, reversing
changes made to 1f27b91.
@10hendersonm 10hendersonm mentioned this pull request Jun 7, 2021
Merged
6 tasks
hipstersmoothie added a commit that referenced this pull request Jun 8, 2021
@hipstersmoothie
Merge pull request #2010 from 10hendersonm/revert_1997
3f1752f 
Revert #1997
@adierkens
Copy link
Collaborator

🚀 PR was released in v10.29.3 🚀

@adierkens adierkens added the released This issue/pull request has been released. label Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
patch Increment the patch version when merged released This issue/pull request has been released.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@10hendersonm @rconnamacher @adierkens @hipstersmoothie