-
Notifications
You must be signed in to change notification settings - Fork 12
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
Delete .npmrc, create it on the fly in CI #29
Conversation
6468a4e
to
da1c82f
Compare
For local development we assume that npm is already properly configured. Removing the bundled dot file avoids overriding personal configurations.
da1c82f
to
31b9c8b
Compare
- name: Create .npmrc | ||
run: | | ||
echo '@restatedev:registry=https://npm.pkg.github.com/' > ~/.npmrc | ||
echo '//npm.pkg.github.com/:_authToken=${GH_PACKAGE_READ_ACCESS_TOKEN}' >> ~/.npmrc | ||
- run: npm ci --prefix typescript | ||
env: | ||
GH_PACKAGE_READ_ACCESS_TOKEN: ${{ secrets.GH_PACKAGE_READ_ACCESS_TOKEN }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @pcholakov, that's a nice improvement!
I think that you need to add that env variable to the step, or alternatively, interpose ${{ secrets.GH_PACKAGE_READ_ACCESS_TOKEN }}
directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we? I think we want the literal content:
@restatedev:registry=https://npm.pkg.github.com/
//npm.pkg.github.com/:_authToken=${GH_PACKAGE_READ_ACCESS_TOKEN}
to land up in .npmrc
without any interpolation; the next step is going to use this and that's the step that already has the env variable set. npm
will do the expansion when npm ci
runs. I'm going to try move it from ~/.npmrc
to typescript/.npmrc
to see if that fixes it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah gotcha! This would also work 👌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as it works 😅 I figured even though GHA executions are ephemeral, it's better to keep the secret in memory and not write it out into the config file.
For local development we assume that npm is already properly configured. Removing the bundled dot file avoids overriding personal configurations.