-
Notifications
You must be signed in to change notification settings - Fork 206
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
Begin codifying the use of NPM dist-tags #4117
Conversation
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.
I have a few questions. Nothing critical.
Our release notes still suffer quite a bit from lax use of conventional commits, i.e. not writing each commit message consciously as a contribution to the release notes.
/******/ ]); |
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.
TIL: we keep copies of yarn releases in our tree
packages/ERTP/CHANGELOG.md
Outdated
|
||
* **ERTP:** NatValues now only accept bigints, lower-case amountMath is removed, and AmountMath methods always follow the order of: brand, value | ||
|
||
* chore: fix up INPUT_VALIDATON.md |
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.
Odd looking breaking change
packages/SwingSet/CHANGELOG.md
Outdated
|
||
### ⚠ BREAKING CHANGES | ||
|
||
* remove newSwap; replace with constantProduct AMM where needed (#4097) |
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.
In the swingset package, removing newswap doesn't seem like a breaking change. I bet it's just some test reorg that ideally would have been a separate commit. I suppose I reviewed this one but didn't consider how squashing would impact release notes
@@ -57,5 +57,8 @@ | |||
"test/**/test-*.js" | |||
], | |||
"timeout": "2m" | |||
}, | |||
"publishConfig": { | |||
"access": "public" |
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.
Commit message seems like it could be more specific
c2aea65
to
3838b3e
Compare
Also, pivot to `latest` NPM dist-tag.
e6dfb2a
to
9ba832e
Compare
# Adapted from https://johnny.sh/notes/publish-canary-lerna-cicd/ | ||
- name: configure NPM token | ||
run: | | ||
echo "//registry.npmjs.org/:_authToken=${NPM_TOKEN}" > ~/.npmrc |
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.
NPM_TOKEN
looks like a very pointy instrument: it seems to represent authority to update all our packages in npm.
I'd like to get @warner 's take on this.
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.
Please move this discussion to #4120.
looks like this refs: #3857 |
4532ba9
to
6ee4e6e
Compare
refs: #3857
Description
Begin distinguishing between
latest
,dev
(and hopefully soonbeta
) tags that users canagoric install latest
in order to keep their dapp dependencies up-to-date.This will make dapp dependency management more like typical Yarn. We also add a CI
dev-canary
publish on push to master, so thatagoric install dev
will get the freshest code.Drive-by: try incorporating the Node 16
CXXFLAGS
support discovered by @robert-zaremba.Security Considerations
Documentation Considerations
Testing Considerations