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 automated published branch releases for dev & main #10520

Merged
merged 13 commits into from May 23, 2024

Conversation

danielbarion
Copy link
Contributor

@danielbarion danielbarion commented May 8, 2024

Description of change

I recently implemented a beta-release workflow on the react-tooltip project, maybe it can be interesting for Pixi.js project too.

On ReactTooltip project, we always try to release a beta version on some PR before merging it to let the owner of the issue test and confirm if the issue was solved, this also unblocks projects that are waiting for some fix before we release an official version with the fix.

With the beta release workflow, all commits into the PR will trigger the beta release.

  • The beta release always releases a new beta version, following this format:
    pixi.js@{__VERSION__}-beta.{__PR_NUMBER__}.rc.{__BETA_VERSION__} -> pixi.js@v0.0.11-beta.1.rc.1
    __VERSION__ - current package.json version on this PR
    __PR_NUMBER__ - current PR number
    __BETA_VERSION__ - latest beta version released
  • to skip the workflow we need to add the following words to the commit message:
[skip ci]
[ci skip]
[no ci]
[skip actions]
[actions skip]

https://docs.github.com/en/actions/managing-workflow-runs/skipping-workflow-runs

Examples

ReactTooltip/react-tooltip#1179 (comment)

image

https://github.com/ReactTooltip/react-tooltip/actions/runs/9007001657/job/24745795089?pr=1179

The previous beta version is null because that's the first beta release for this PR number.
image

https://www.npmjs.com/package/react-tooltip?activeTab=versions

image

Note: ReactTooltip master branch has the CJS code (same as this PR), on V6 branch we have it as ESM version of the code.

Copy link

korbit-ai bot commented May 8, 2024

My review is in progress 📖 - I will have feedback for you in a few minutes!

Copy link

codesandbox-ci bot commented May 8, 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.

Latest deployment of this branch, based on commit a194cab:

Sandbox Source
pixi.js-sandbox Configuration

Copy link

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

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

I have reviewed your code and found 1 potential issue. To discuss my individual comments that I have added, tag me in replies using @korbit-ai.


Please react with a 👍 to my comments that you find helpful and a 👎 to those you find unhelpful - this will help me learn and improve as we collaborate.

beta-release.js Outdated Show resolved Hide resolved
@bigtimebuddy
Copy link
Member

bigtimebuddy commented May 15, 2024

@danielbarion We (@Zyie @GoodBoyDigital and I) had a chance to review this change this morning. Overall we love the general idea and thank you for proposing it! Super useful feature to opt-in to bleeding-edge unreleased changes.

@Zyie pointed out that our codesandbox integration offers some of this (at least testing purposes) for PRs and there is also a mechanism to download a build (I think) using that tool.

That said, we still think there's something here and we have some feedback. If you don't have time to work on these, just let us know someone on the team will likely pick it up.

  • Rather than focus on PRs, would like to provide automated releases for unreleased but merged functionality. It's common feedback we get that sometimes users have to wait awhile for a release. This would help elevate that pressure.
  • We'd like to not use the term "beta" here for these release, we use "beta" semver for specific, intentional tagged releases.
  • Preference would be to have versions formatted as: 8.1.0-[branch].[commit-hash], for instance 8.1.3-dev.9afd9d3. The benefit of doing this is that versions would tie directly to the branch and commit. That's super helpful for debugging and triaging errors.
  • Preference for npm dist-tags to use the branch name, e.g., dev, that way users could define "pixi.js": "dev" in their dependencies or npm install pixi.js@dev
  • We would only like to focus on main and dev branches as these are the only branches we cut releases from.

What do you think?

@danielbarion
Copy link
Contributor Author

danielbarion commented May 15, 2024

Hey @bigtimebuddy & team, it does make sense! the initial implementation is the same as we have on ReactTooltip that matches our needs.

Thank you so much for reviewing this suggestion and for the detailed feedback, I really appreciate that!

I would like to focus on those changes so the Pixi team can concentrate on other stuff, I have a test repository where I developed this workflow initially to not release the test versions directly on the ReactTooltip project, I'll use it to apply the needs mentioned on your feedback.

I'll keep you guys in the loop.

Thanks!

Edit:

What should be the name of the workflow? Release Candidate instead of Beta Release?

@bigtimebuddy
Copy link
Member

Maybe, Publish Branch?

.github/workflows/publish-branch.yaml Outdated Show resolved Hide resolved
.github/workflows/publish-branch.yaml Outdated Show resolved Hide resolved
@danielbarion
Copy link
Contributor Author

image

image

https://www.npmjs.com/package/auto-beta-release?activeTab=versions

The code was updated, please let me know if something else is needed, I didn't update the PR description because of our comments, but please let me know your thoughts about it and about the changes

.github/workflows/publish-branch.yaml Outdated Show resolved Hide resolved
.github/workflows/publish-branch.yaml Outdated Show resolved Hide resolved
.github/workflows/publish-branch.yaml Outdated Show resolved Hide resolved
.github/workflows/publish-branch.yaml Outdated Show resolved Hide resolved
.github/workflows/publish-branch.yaml Outdated Show resolved Hide resolved
update-branch-version.js Outdated Show resolved Hide resolved
@bigtimebuddy
Copy link
Member

@danielbarion I did a quick experiment here that seemed to work for me:

https://github.com/bigtimebuddy/actions-branch-version/blob/a68fa58bdbc26d9e7ef8e662f78aff09aab89a62/.github/workflows/main.yml#L34

Added two args to npm version:

  • --no-git-tag-version - Ignores git tagging which means that it doesn't require git auth/identity
  • --force - Bump the version even if the git workspace is dirty or has uncommitted changed, this wasn't necessary, but will help prevent issues with, say, npm updates that change the package-lock.json

Co-authored-by: Matt Karl <matt@mattkarl.com>
@danielbarion
Copy link
Contributor Author

@bigtimebuddy hmm, interesting, maybe it's the action I was using? I saw the project is using v3 and I was using v4 initially (uses: actions/setup-node@v3), or maybe the setup I did previously.

Thanks for letting me know

@danielbarion
Copy link
Contributor Author

Sorry, I asked review again but I forgot to apply the change, 1 second.

@danielbarion
Copy link
Contributor Author

Updated

Copy link
Member

@bigtimebuddy bigtimebuddy left a comment

Choose a reason for hiding this comment

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

Could you revert the package.json and package-lock and I think we are good to go!

@danielbarion
Copy link
Contributor Author

@bigtimebuddy NPM sorted the package by itself, should I remove it too?

Copy link
Member

@bigtimebuddy bigtimebuddy left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

Copy link
Member

@bigtimebuddy bigtimebuddy left a comment

Choose a reason for hiding this comment

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

Also, are we even building before the publish?

.github/workflows/publish-branch.yaml Outdated Show resolved Hide resolved
@bigtimebuddy bigtimebuddy changed the title [CI Workflow] feat: automated beta release for pull requests feat: add automated published branch releases for dev & main May 17, 2024
@Zyie
Copy link
Member

Zyie commented May 23, 2024

@bigtimebuddy @danielbarion is everything all settled with this PR now?

@danielbarion
Copy link
Contributor Author

yes @Zyie

@bigtimebuddy
Copy link
Member

Yeah, I'm good on this. I think we should merge this into dev and just validate that everyone is working as expected.

@Zyie Zyie added this pull request to the merge queue May 23, 2024
Merged via the queue into pixijs:dev with commit 141f7e0 May 23, 2024
4 checks passed
@danielbarion
Copy link
Contributor Author

@Zyie @bigtimebuddy

image

@Zyie
Copy link
Member

Zyie commented May 23, 2024

Ok this seems to be working!

https://www.npmjs.com/package/pixi.js/v/8.1.5-dev.141f7e0?activeTab=versions

Thank you @danielbarion for adding this

@danielbarion
Copy link
Contributor Author

@Zyie do you want me to do the PR to main?

@Zyie
Copy link
Member

Zyie commented May 23, 2024

@Zyie do you want me to do the PR to main?

no it's ok, this will get merged into main when i do the next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants