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

Ensure npm credentials are set by publish_snapshot.sh #422

Merged
merged 5 commits into from
Mar 23, 2023

Conversation

jeremywiebe
Copy link
Collaborator

Summary:

We've seen Github action failures in the "Publish npm snapshot" job. This is because the npm auth info has not been provided (this is done automatically by the changesets Github Action, but we aren't using it here because we're publishing a snapshot.

This PR mimics what the changesets Github Action does by creating a .npmrc file with the credentials in it.

Issue: "none"

Test plan:

Create a new PR after landing this one.
Ensure that the "Publish npm snapshot" job succeeds.

@jeremywiebe jeremywiebe self-assigned this Mar 23, 2023
@changeset-bot
Copy link

changeset-bot bot commented Mar 23, 2023

🦋 Changeset detected

Latest commit: 6f7d9ba

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Mar 23, 2023

npm Snapshot: NOT Published

🤕 Oh noes!! We couldn't find any changesets in this PR (a8fbdbf). As a result, we did not
publish an npm snapshot for you.

@codecov
Copy link

codecov bot commented Mar 23, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@1fe3786). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head a2e9509 differs from pull request most recent head 6f7d9ba. Consider uploading reports for the commit 6f7d9ba to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #422   +/-   ##
=======================================
  Coverage        ?   65.98%           
=======================================
  Files           ?      487           
  Lines           ?   107108           
  Branches        ?     7376           
=======================================
  Hits            ?    70674           
  Misses          ?    36434           
  Partials        ?        0           

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1fe3786...6f7d9ba. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 23, 2023

Size Change: 0 B

Total Size: 635 kB

ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 38.4 kB
packages/kmath/dist/es/index.js 4.18 kB
packages/math-input/dist/es/index.js 60.2 kB
packages/perseus-editor/dist/es/index.js 112 kB
packages/perseus-error/dist/es/index.js 825 B
packages/perseus-linter/dist/es/index.js 18.6 kB
packages/perseus/dist/es/index.js 385 kB
packages/pure-markdown/dist/es/index.js 3.74 kB
packages/simple-markdown/dist/es/index.js 12.9 kB

compressed-size-action

exit 1
fi

verify_env() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of lines moved here. I've gathered bits of functionality into functions so it's a bit less of a "massive wall of procedural functionality". Now if you skip to line 107 you can follow the functionality more easily (I hope!). The new lines are in create_npmrc (lines 88-99)!

@jeremywiebe jeremywiebe marked this pull request as ready for review March 23, 2023 19:36
@jeremywiebe jeremywiebe requested review from a team and handeyeco March 23, 2023 19:36
@khan-actions-bot
Copy link
Contributor

Gerald

Required Reviewers
  • @Khan/perseus for changes to .changeset/yellow-garlics-return.md, utils/publish-snapshot.sh

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

@jeremywiebe jeremywiebe changed the title Ensure NPM_TOKEN is set when calling publish_snapshot.sh Ensure npm credentials are set by publish_snapshot.sh Mar 23, 2023
Copy link
Contributor

@handeyeco handeyeco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like Bash to me 👍

create_npmrc


exit
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this exit here prevent the following lines from running? Like yarn build and yarn extract-strings?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yikes! Thanks for catching this. I'd added it to test something locally. Removed!!!

@jeremywiebe jeremywiebe merged commit a18ef7e into main Mar 23, 2023
@jeremywiebe jeremywiebe deleted the fix-npm-snapshot branch March 23, 2023 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants