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.

Sign up for GitHub

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

feat(publib-npm): allow dry run and support _auth token type #821

Merged
merged 2 commits into from
Jul 27, 2023
Merged

feat(publib-npm): allow dry run and support _auth token type #821

merged 2 commits into from
Jul 27, 2023

Conversation

eknowles
Copy link
Contributor

@eknowles eknowles commented Jul 15, 2023 •
edited by mrgrain
Loading

This PR fixes 3 problems with NPM publishing

  1. No support for other repositories (that use _auth instead of _authToken) e.g. Jfrog (closes publib-npm only uses authToken #819)
  2. No support for dry-run option, this is available in Go but not npm. (closes no dry-run option for npm #820)
  3. NPM publish will overwrite the users .npmrc, this has been switched out to use a temporary directory.
  4. NPM publish did not clean up npmlog.txt on successful runs, this PR will cleanup the npmrc and log files.

Added two new options for publib-npm

  • NPM_DRYRUN (optional): Set to "true" for a dry run
  • NPM_AUTH_TYPE (optional): Can be "authToken" (default) or "auth" depending on the type of NPM_TOKEN used for the NPM_REGISTRY

@eknowles eknowles had a problem deploying to IntegTestCredentialsRequireApproval July 15, 2023 22:40 — with GitHub Actions Failure
auto-merge was automatically disabled July 15, 2023 22:47

Head branch was pushed to by a user without write access

@eknowles eknowles had a problem deploying to IntegTestCredentialsRequireApproval July 15, 2023 22:47 — with GitHub Actions Failure
@eknowles eknowles mentioned this pull request Jul 15, 2023
Closed
eknowles added 2 commits July 15, 2023 23:48
@eknowles
feat(publib-npm): allow dry run and support _auth token type
06cd88d 
.npmrc are also created in a temp dir so as to not
overwrite existing npmrc in home directory

fixes cdklabs#820 cdklabs#819
@eknowles
docs(readme): update publib-npm options
dfafb8c
@eknowles eknowles temporarily deployed to IntegTestCredentialsRequireApproval July 15, 2023 22:49 — with GitHub Actions Inactive
@johntaormina
Copy link

Following this change as publishing with _auth is something I would love to see 👍

@mrgrain
Copy link
Contributor

mrgrain commented Jul 18, 2023

Thanks @eknowles for the contribution. Got your email as well. 👍🏻
PR looks good but it will take a while for me to get to it and verify everything works. Unfortunately test coverage isn't great for this tool. You could speed things up by adding a couple of integration tests for your scenarios.

@eknowles
Copy link
Contributor Author

I saw there wasn't any tests around the shell scripts but was on typescript, would you prefer me to test the shell or rewrite in ts?

@mrgrain
Copy link
Contributor

mrgrain commented Jul 19, 2023 •
edited
Loading

I saw there wasn't any tests around the shell scripts but was on typescript, would you prefer me to test the shell or rewrite in ts?

I think the goal would be to have an integration test for each shell script entrypoint. The test/publib-ca.integ.ts is fairly new, but should be able to serve as a guideline. I appreciate this is a big ask but would certainly help. I will have more time to invest into this late next week (please ping me if I don't get to it).

@mrgrain mrgrain added this pull request to the merge queue Jul 27, 2023
Merged via the queue into cdklabs:main with commit 41e5605 Jul 27, 2023
mrgrain added a commit that referenced this pull request Jul 27, 2023
@mrgrain
Revert "feat(publib-npm): allow dry run and support _auth token type (
b9f726c 
#821)"

This reverts commit 41e5605.
@mrgrain mrgrain mentioned this pull request Jul 27, 2023
Merged
@mrgrain
Copy link
Contributor

mrgrain commented Jul 27, 2023

@eknowles Yeah that doesn't work. And I didn't test enough 😬

mrgrain added a commit that referenced this pull request Jul 27, 2023
@mrgrain
chore: revert "feat(publib-npm): allow dry run and support _auth to…
31934b5 
…ken type" (#835)

Revert "feat(publib-npm): allow dry run and support `_auth` token type (#821)"

This reverts commit 41e5605.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrgrain mrgrain mrgrain approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

no dry-run option for npm publib-npm only uses authToken
3 participants
@eknowles @johntaormina @mrgrain