-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
chore(.github/workflows): improve workflow with removing duplicated configs #2918
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
commit: |
.github/workflows/cr.yml
Outdated
- uses: actions/setup-node@v4 | ||
with: | ||
node-version: 20 | ||
cache: 'pnpm' | ||
- run: pnpm install | ||
cache-dependency-path: '**/pnpm-lock.yaml' |
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.
Default works, so we don't need it. Let's remove it from other *.yml
.
.github/workflows/cr.yml
Outdated
- uses: actions/setup-node@v4 | ||
with: | ||
node-version: 20 | ||
cache: 'pnpm' | ||
- run: pnpm install | ||
cache-dependency-path: '**/pnpm-lock.yaml' | ||
- run: pnpm install --frozen-lockfile |
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.
frozen-lockfile is the default in CI, so let's remove it. https://github.com/actions/setup-node/blob/main/docs/advanced-usage.md#caching-packages-data
@dai-shi Are you saying that it might not be necessary to specify caching in detail in |
No, it works without specifying it because it's the default behavior. |
@dai-shi I was aware that pnpm caching had already been specified and utilized prior to this PR. However, I thought it would look cleaner to ensure consistency with other YML files. |
yeah, i wasn't aware that for a long time. so, i'm suggesting to make it consistent by removing it in other files. |
name: Publish Any Commit
on: [push, pull_request]
jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: pnpm/action-setup@v4
- uses: actions/setup-node@v4
with:
node-version: 20
cache: 'pnpm'
- run: pnpm install
- run: pnpm build
- run: pnpm dlx pkg-pr-new publish './dist' --compact --template './examples/*' @dai-shi i understood, you want to simplify like this? by including only what is absolutely necessary. |
Yes! |
@dai-shi cool!, i'll do it all in this pr, includes jotai, valtio |
d396c27
to
9ca8586
Compare
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 update the PR description.
@dai-shi Done! |
Thanks for your work for all three repos. I like it. |
Summary
Check List
pnpm run prettier
for formatting code and docs