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: add pkg.pr.new #17314

Merged
merged 21 commits into from
Jun 11, 2024
Merged

feat: add pkg.pr.new #17314

Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
8afe458
add pkg.pr.new
AmirSa12 May 25, 2024
e4df2cc
add names for linked packages
AmirSa12 May 25, 2024
0e7346c
using pnpm
AmirSa12 Jun 3, 2024
b6450fe
empty commit
AmirSa12 Jun 3, 2024
4e21134
empty commit
AmirSa12 Jun 4, 2024
d0fa5fd
remove types
AmirSa12 Jun 4, 2024
b71be09
compact mode and approved prs
AmirSa12 Jun 4, 2024
5c1d979
empty commit
AmirSa12 Jun 5, 2024
04fd31e
remove gh token
AmirSa12 Jun 7, 2024
593d0fb
test
AmirSa12 Jun 7, 2024
a761804
test
AmirSa12 Jun 7, 2024
d809a48
no compact
AmirSa12 Jun 7, 2024
32f3ff8
per approved PRs
AmirSa12 Jun 7, 2024
99fe173
test
AmirSa12 Jun 7, 2024
f9dd3c5
test
AmirSa12 Jun 7, 2024
eb67bfe
approved prs & no compact
AmirSa12 Jun 7, 2024
c13932e
approved prs & no compact
AmirSa12 Jun 7, 2024
09a3bc2
workflows disabled in fork
AmirSa12 Jun 9, 2024
3514392
ok
AmirSa12 Jun 9, 2024
bbe441b
remove names
AmirSa12 Jun 10, 2024
77b7137
publish-commit
AmirSa12 Jun 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions .github/workflows/cr.yml
View file
Open in desktop
bluwy marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
name: Publish Any Commit
on:
push:
branches:
- main
- v6/environment-api
pull_request_review:
types: [submitted]

jobs:
build:
if: github.event_name == 'push' || github.event.review.state == 'APPROVED'
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@v3

- run: corepack enable
bluwy marked this conversation as resolved.
Show resolved Hide resolved
- uses: actions/setup-node@v4
with:
node-version: 20
cache: "pnpm"

- name: Install dependencies
run: pnpm install

- name: Build
run: pnpm build

- run: pnpx pkg-pr-new publish --compact --pnpm ./packages/vite
Copy link
Member

Choose a reason for hiding this comment

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

pnpm dlx is now recommended over pnpx. Could we use it instead?

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I forgot @dominikg's feedback. @AmirSa12 we should move to pkg-pr-new being a dev dependency, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

that would be great, yes. Another option would be to use an explicit version here pnpm dlx [email protected]

Also if this only publishes vite, then maybe it's cheaper to run build only in packages/vite?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@patak-dev we can do that, we were working on issues so we used latest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dominikg, Thanks! Yes lets run build only in packages/vite since we only publish that

Copy link
Member

Choose a reason for hiding this comment

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

I think we should go with pnpm dlx [email protected] instead of the dev dependency, so we don't get this locked at the patch level in our package lock. This should give us the latest 0.0.x version IIUC, that would be useful now that the project is releasing bug fixes often.

Copy link
Contributor

Choose a reason for hiding this comment

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

one thing you can add to enhance security even in the event the token is stolen is

https://github.com/sveltejs/vite-plugin-svelte/blob/45a43a4084499559a2e7a08b2403729e835fa06a/.github/workflows/ci.yml#L20

https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs

with that, the script not needing the token by default and the fact that we know and trust the org that controls the pkg-pr-new package it could be fine to keep this unpinned if it helps development.

But i kind of wonder if it is a good idea to automate it already if it is still undergoing frequent changes. Wouldn't a comment trigger like with ecosystem-ci be a slower start where you can learn about what works?

1 change: 1 addition & 0 deletions packages/vite/src/types/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"//": "this file is just here to make pnpm happy with --frozen-lockfile",
"name": "dep-types",
AmirSa12 marked this conversation as resolved.
Show resolved Hide resolved
"version": "0.0.0"
}
1 change: 1 addition & 0 deletions packages/vite/types/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"//": "this file is here to make typescript happy when moduleResolution=node16+",
"name": "types",
AmirSa12 marked this conversation as resolved.
Show resolved Hide resolved
"version": "0.0.0"
}