feat: add pkg.pr.new
Description
This PR adds pkg.pr.new to the repo
Run & review this pull request in StackBlitz Codeflow.
we encountered an issue with npm and we are trying to solve it. I marked this as a draft PR until then.
EDIT: created a PR related to an upstream issue in npm npm/cli#7579. Currently, we have decided to switch to pnpm for this case.
Thanks @AmirSa12! I'll discuss with the rest of the team about this PR once it the issue is solved 👍🏼
@patak-dev, the app is not installed on this repo.
Not opposing this addition as I'd like to support preview releases too, but is there a reason for a GitHub app for this? Couldn't it fit entirely in CI with a secret token, like publishing to npm?
is there a reason for a GitHub app for this
The github app is used mainly for authentication and avoiding spam publishes.
Couldn't it fit entirely in CI with a secret token, like publishing to npm?
One of the characteristics of pkg.pr.new is that it does not publish to npm so it does not make the npm history ugly and instead publishes the package to the pkg.pr.new domain. Which means way more control and flexibility for later removals for instance.
IIUC the GitHub app would be the way to authenticate and start using pkg.pr.new, and no other sign in is required? I can get by a GitHub app in that case 👍
I get that it's not relying on npm, but I thought it was sort of a js registry so it could have a similar auth flow as npm. I guess it may not be possible to publish locally in the future, but perhaps it's intentionally out of scope.
no other sign in is required
Yes, nothing else is required!
I guess it may not be possible to publish locally in the future
You got it perfectly right, it's not a js registry. Let's say it's an npm tarball/artifact host 😅
The app is now installed @AmirSa12. Great to see you fixing that npm bug upstream!
@AmirSa12 @Aslemammad about the generated message, is this generated only once per PR and then updated? Some comments about it:
- We already have the StackBlitz app, so it would be good to check if there is already a
Review PRbutton and avoid adding another one. - If
Open in Codeflowis exactly the same as the Review PR one, we should align their sizes and use the same wording in general - We probably don't need the types and dep-types lines here, no? Probably a bug on pkg.pr.new side
- Maybe a good idea to detect what package manager the project is using and use that instead of always
npmin the code snippet. - It is taking too much space in the discussion in general, even if it is a single one and the above are removed. I would expect something closer to
Titles could be smaller too, image just for reference.
About the PR vs commit hash, I wonder if a single URL with the PR would be good enough here. This is what normally you'd want to use in a PR. So in that case, it would be even better. If that is the case, including the title with the PR number is not needed as we are already in that PR. So it could be just
Thank you for the helpful suggestions, we'll address as many as possible and keep you updated here.
waiting for approval to see if everything is working as expected.
Thank you! Everything seems to be working fine.
Should we add the last modified date too? Right now it requires the user to click on the Edited dropdown to see that. Maybe this is ok though. I'm good with moving forward without it.
The hash link is currently pointing to https://github.com/vitejs/vite/runs/26016615176 That page doesn't link to the commit inside. Is this intentional? I was expecting a direct link to the commit in the bot message.
Ok, I discussed with @Aslemammad and having the date is challenging due to time zones (something to look for later). And the current link has the commit name and link when you navigate to it, so I'm good with that too.