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.

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

feat: add pkg.pr.new #898

Merged
merged 4 commits into from
May 20, 2024
Merged

feat: add pkg.pr.new #898

merged 4 commits into from
May 20, 2024

Conversation

AmirSa12
Copy link
Contributor

Summary

This PR adds pkg.pr.new

Copy link

vercel bot commented May 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
valtio ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 20, 2024 6:20am

Copy link

codesandbox-ci bot commented May 19, 2024

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.

@AmirSa12
Copy link
Contributor Author

OK we still have the same issue, even after opening a new PR. ( the check fails at first then it goes green automatically after few seconds)

@AmirSa12
Copy link
Contributor Author

AmirSa12 commented May 19, 2024

OK now we know the reason. according to PR #896 , the reason the check failed at first was that the workflows were running in my fork and I didn't install the app myself, so it failed on my fork and the result were being shared with you all !! @dai-shi I apologize for the confusions caused by my emails :)

Copy link

pkg-pr-new bot commented May 19, 2024

Last Commit Build: 349cf69

valtio(349cf69):

npm i https://pkg.pr.new/valtio@349cf69    

Pull Request Build: #898

valtio(#898):

npm i https://pkg.pr.new/valtio@898    

@dai-shi
Copy link
Member

dai-shi commented May 19, 2024

I don't receive the email, which is good. Thanks.

Now, I get this error:

$ npm i https://pkg.pr.new/pmndrs/valtio@99014aa    
npm ERR! code E404
npm ERR! 404 Cannot find any route matching /pmndrs/valtio@99014aa. - GET https://pkg.pr.new/pmndrs/valtio@99014aa
npm ERR! 404 
npm ERR! 404  'https://pkg.pr.new/pmndrs/valtio@99014aa' is not in this registry.
npm ERR! 404 This package name is not valid, because 
npm ERR! 404  1. name can only contain URL-friendly characters
npm ERR! 404 
npm ERR! 404 Note that you can also install from a
npm ERR! 404 tarball, folder, http url, or git url.

npm ERR! A complete log of this run can be found in: /Users/daishi/.npm/_logs/2024-05-19T22_26_04_669Z-debug-0.log

@dai-shi
Copy link
Member

dai-shi commented May 19, 2024

@Aslemammad I don't know the technical detail, but it would be super nice if it didn't require the app. Why github action isn't enough if you have a backend server?

@Aslemammad
Copy link
Member

Aslemammad commented May 20, 2024

Now, I get this error:

I'll try fixing this asap! Sorry for the error.

but it would be super nice if it didn't require the app

Yep, but we need to listen to PR & commit events so we validate if a certain workflow is a verified workflow. This also helps in avoiding spam publishes or local publishes. The app is also required for commenting.
Let me know if I'm wrong, it'd be really good to avoid friction by removing the app.

@Aslemammad
Copy link
Member

The issue is upstream, sorry for that! Feel free to convert the PR to draft until my fix get merged.

@Aslemammad
Copy link
Member

But we can try compact mode too! Let's try that meanwhile.

@dai-shi
Copy link
Member

dai-shi commented May 20, 2024

What's compact mode?

@Aslemammad
Copy link
Member

--compact makes the urls way shorter if the package exists on npm!

like:
npm i https://pkg.pr.new/valtio@349cf69, no repo & org name!

@Aslemammad
Copy link
Member

It works now, let me know what you think Daishi!

@dai-shi
Copy link
Member

dai-shi commented May 20, 2024

Not sure how to deal with name collisions, but I don't mind.
Let's merge it and see how it goes in this repo. It would be nice if it creates some sandboxes too.

@dai-shi dai-shi merged commit 6929465 into pmndrs:main May 20, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants