Skip to content

install pkg.pr.new github workflow#214

Open
TomKaltz wants to merge 2 commits intoSuperFlyTV:mainfrom
TomKaltz:tk-pkg-pr-new
Open

install pkg.pr.new github workflow#214
TomKaltz wants to merge 2 commits intoSuperFlyTV:mainfrom
TomKaltz:tk-pkg-pr-new

Conversation

@TomKaltz
Copy link
Contributor

@TomKaltz TomKaltz commented May 19, 2025

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Add's https://pkg.pr.new workfow so users can test unreleased builds or test PR's before they are merged.

  • What is the current behavior? (You can also link to an open issue here)
    Users have to clone and build if they want to test unreleased features

  • What is the new behavior (if this is a feature change)?
    Users can now check the action output for a url that they can use to install a specific build of this package. For PR's a comment will be added to the PR thread with a url for the built package.

  • Other information:
    A maintainer must also install/authorize the github app for this org. See https://pkg.pr.new for instructions.

Comment on lines +21 to +22
corepack prepare yarn@3.8.3 --activate
yarn set version 3.8.3
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason for pinning an exact yarn version here, rather than letting corepack handle it in the usual way (with simply corepack enable)?

"@types/xml2js": "^0.4.11",
"jest": "^29.4.3",
"open-cli": "^7.1.0",
"pkg-pr-new": "^0.0.50",
Copy link
Member

Choose a reason for hiding this comment

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

I dont think this needs to be here. The workflow is using yarn dlx which will download a temporary version of the package, and I think it will ignore this version entirely. So this being here serves no purpose?

Copy link
Member

@Julusian Julusian left a comment

Choose a reason for hiding this comment

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

I can see the value of this, and it isnt very invasive, so I am open to including this.

I have not heard of this tool before though, so am a bit cautious of it..

So on that note, I think it would be good to limit the permissions of the workflow by adding:

permissions:
  contents: read

at the root level, just so that I don't have to worry about about what the package might be doing


steps:
- name: Checkout code
uses: actions/checkout@v4
Copy link
Member

Choose a reason for hiding this comment

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

In the same way of limiting permissions of this, I think this should have the persist-credentials: false option added

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.

2 participants