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

Rewrite preview action in Node #65

Merged
merged 50 commits into from
Nov 15, 2021
Merged

Rewrite preview action in Node #65

merged 50 commits into from
Nov 15, 2021

Conversation

minkimcello
Copy link
Contributor

@minkimcello minkimcello commented Oct 5, 2021

Motivation

To finally rewrite the publish-pr-preview action in typescript.

Approach

What's New?

  • ⛔ Removed IGNORE and NPM_PUBLISH options
    • IGNORE was originally for us to prevent publishing our gatsby/docusaurus website directories but we have them marked as private packages and the action will skip over private packages anyway
    • NPM_PUBLISH was to give developers the option to use yarn publish but I think we're just using npm publish so it's not necessary for us to have this option... I think
  • ⛔ We no longer create/append .npmrc files. We're letting actions/setup-node take care of all of that
    • The steps that are necessary prior to running the preview action is outlined in the README
  • ⚠️ Replaced BEFORE_ALL with INSTALL_SCRIPT
    • Previously, we ran yarn install and then BEFORE_ALL before proceeding on to publishing but now we just combined the two so it's either just yarn install or user-specified install script. This just means users need to configure INSTALL_SCRIPT: yarn install && yarn other stuff if they need to run additional steps
  • ⚠️ We used to transform branch names to tags by replacing _ with __ and / with _ but I found out underscore is not valid for semver prerelease identifiers so we're now replacing both _ and / with - instead
  • ❇️ New preview versioning
    • Old = 1.0.0 => 1.0.0-abcdefgh (SHA)
    • New = 1.0.0 => 1.0.1-branch.0 (branch name as prerelease identifier with increments)
      • The action will look at the npm registry to grab the highest non-major version of your package and do a prerelease bump from there. So locally my package might say 1.0.0 but if 1.2.3 was released and I haven't had a chance to rebase yet, the action will publish 1.2.4-branch.0.
  • ❇️ Better comment format
    • copy button for convenience
    • footer with frontside logo and link to the source code of action
    • yarn add if there's yarn.lock in project, npm install by default
      Screen Shot 2021-11-02 at 11 56 35 AM

Publishing Tested

  • 1. 1.15.0 published 1.15.1-mk-preview-js.0
  • 2. ran again and it published 1.15.1-mk-preview-js.1
    • action ran npm view tag to detect that .0 was already published so it incremented up to .1
  • 3. manually published 1.16.0 and ran action and it published 1.16.1-mk-preview-js.0 from 1.15.0
    • starts from .0 again because we never published a preview for 1.16.1-x
  • 4. manually published 1.16.1-mk-preview-js.1 and forced increaseFrom as 1.16.0 to test multiple attempt functionality
    • this published 1.16.1-mk-preview-js.2 after attempting to publish .0 and .1
  • 5. ran again and didn't publish anything after trying .0, .1, and .2
  • 6. undid the forced increaseFrom of 1.16.0 and action published 1.16.1-mk-preview-js.3

TODOs

Once this PR is merged, we can use this action by calling it from the main branch:

uses: thefrontside/actions/publish-pr-preview@main

But I would like to rewrite the other actions too so that we can officially release everything as v2.

@minkimcello minkimcello force-pushed the mk/preview-js branch 2 times, most recently from 89af8ba to 750b213 Compare October 5, 2021 13:59
@minkimcello minkimcello force-pushed the mk/preview-js branch 6 times, most recently from 497bebb to cc69151 Compare October 12, 2021 21:21
@minkimcello minkimcello force-pushed the mk/preview-js branch 2 times, most recently from 2afd33f to 8728263 Compare October 12, 2021 22:34
@minkimcello minkimcello force-pushed the mk/preview-js branch 5 times, most recently from 4fac799 to 74aad74 Compare November 3, 2021 16:42
@minkimcello minkimcello force-pushed the mk/preview-js branch 2 times, most recently from 50127d7 to e54b243 Compare November 3, 2021 17:57
cowboyd
cowboyd previously approved these changes Nov 4, 2021
Copy link
Member

@cowboyd cowboyd left a comment

Choose a reason for hiding this comment

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

Is this almost ready to go? Curious. How do we handle the publishing based on the presence of a preview label?

@cowboyd cowboyd dismissed their stale review November 4, 2021 10:07

meant to jut comment

@minkimcello
Copy link
Contributor Author

Is this almost ready to go? Curious. How do we handle the publishing based on the presence of a preview label?

I just need to add some informative console logs here and there and it will be good to go.

In terms of publishing from an existing prerelease, we originally planned for something along the lines of 1.0.0-beta-my_branch.1. However, I found semver doesn't distinguish between prerelease and prerelease-of-a-prerelease. So I could have appended the two identifiers as one (beta-my_branch) or I could just treat the previews as its own "less-official" prerelease. I went with the latter.

  • If we have 1.1.1 and 1.99.99-beta.0, a preview will go off the highest published non-major version (1.1.1) and patch bump and prerelease from there (1.1.2-my_branch.0).
  • If we have 2.0.0-beta.0 and that's the version specified in package.json (with 2.0.0 not yet published), we will still get 2.0.0-my_branch.0.

@minkimcello
Copy link
Contributor Author

Is this almost ready to go? Curious. How do we handle the publishing based on the presence of a preview label?

Oh I forgot to answer your question. This can be done from the workflow. I could've incorporated it into the action but using the workflow prevents having to build the action so we'll save time.

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

2 participants