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(plugins/npm): permit lerna publish to use automation tokens #2032

Conversation

hydrosquall
Copy link
Contributor

@hydrosquall hydrosquall commented Jul 15, 2021

What Changed

  • lerna publish was failing authentication even with a token that clearly had write access, as using yarn npm publish works. As a result, the canary build feature of auto wasn't working.

  • Examining Lerna doesn't work with NPM automation tokens lerna/lerna#2788 revealed the root issue, automation tokens fail the identity check used at the start of lerna publish unless a flag is passed.

  • Updates locations in the auto code where lerna publish is called to not require the identify verification check.

Why

  • Using auto in a monorepo was failing, using a non-automation token is not an option for this project
  • Alternately we could expose some config to opt in/out of having this flag, but I think having good behavior by default for automation tokens is desirable, especially for canary builds.

Todo:

  • Add tests
  • Add docs

Change Type

Indicate the type of change your pull request is:

  • documentation
  • patch
  • minor
  • major

Testing

I'm not sure how to trigger a canary package publish within this repo (maybe a tag is necessary), so I've published a fork of this under my own scope to test separately.

yarn add -D @hydrosquall/auto@10.29.3-cameron-1

@adierkens adierkens added the minor Increment the minor version when merged label Jul 15, 2021
@codecov
Copy link

codecov bot commented Jul 15, 2021

Codecov Report

Merging #2032 (3cfe77a) into main (3f1752f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2032   +/-   ##
=======================================
  Coverage   80.23%   80.23%           
=======================================
  Files          66       66           
  Lines        5394     5394           
  Branches     1249     1249           
=======================================
  Hits         4328     4328           
  Misses        706      706           
  Partials      360      360           
Impacted Files Coverage Δ
plugins/npm/src/index.ts 72.98% <ø> (ø)

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 909ae8b...3cfe77a. Read the comment docs.

@hydrosquall hydrosquall changed the title feat: permit lerna publish to use automation tokens feat(plugins/npm): permit lerna publish to use automation tokens Jul 15, 2021
@hydrosquall
Copy link
Contributor Author

hydrosquall commented Jul 15, 2021

My apologies, while trying to figure out how to publish a single plugin instead of the whole auto CLI, I realized that this option is already documented in the NPM plugin readme.

https://github.com/intuit/auto/tree/main/plugins/npm#using-automation-tokens-from-npm

I still think that supporting this behavior by default may be relevant given that most people are running auto in CI, but recognize that is a breaking change where I'd defer to the project maintainer's judgement/preference.

@zephraph
Copy link
Collaborator

Yeah, I added the documentation after stumbling across this. It's not the best experience. I'd be 👍 for the change.

@hipstersmoothie
Copy link
Collaborator

@hydrosquall I like the change. Could you remove that bit from the docs since it isn't needed anymore?

@hydrosquall
Copy link
Contributor Author

@hydrosquall I like the change. Could you remove that bit from the docs since it isn't needed anymore?

Updated in 3cfe77a

@hipstersmoothie hipstersmoothie merged commit 40f5c49 into intuit:main Aug 12, 2021
@adierkens
Copy link
Collaborator

🚀 PR was released in v10.31.0 🚀

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

Successfully merging this pull request may close these issues.

None yet

4 participants