Skip to content

Add publish step #33

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add publish step #33

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7a826ab
Add publish step
progremir May 19, 2022
f379371
Name version update step
progremir May 19, 2022
d3c846d
Remove dry-run
progremir May 19, 2022
e696dee
Rename workflow file
progremir May 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions .github/workflows/build-website.yml → .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ jobs:

steps:
- name: Checkout 🛎️
uses: actions/checkout@master
uses: actions/checkout@v3
Copy link
Member

@thisisdhaas thisisdhaas May 23, 2022

Choose a reason for hiding this comment

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

Do we know if there was a reason we were pointing at the development branch before / was there a reason you needed to make this change?

Copy link
Contributor Author

@progremir progremir May 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I don't know honestly. It just felt right to point at a specific version, so that things don't break suddenly


- name: Setup Node 🔧
uses: actions/setup-node@v3
with:
node-version: '14'

- name: Install and Build 🔧
run: |
yarn install
Expand All @@ -30,3 +30,16 @@ jobs:
with:
branch: gh-pages # The branch the action should deploy to. It should be any branch other than `main`
folder: dist # The folder the action should deploy.

- name: Publish ✨
Copy link
Member

@thisisdhaas thisisdhaas May 23, 2022

Choose a reason for hiding this comment

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

I know very little about github actions, but this is a 'step' within the 'build' job. Is it better practice to have a 'build' job and a 'publish' job? (That would be closer to our CircleCI setup for other repos)

Copy link
Contributor Author

@progremir progremir May 23, 2022

Choose a reason for hiding this comment

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

That's a good question! I don't know the best practices here, so you tell me.
I chose to make it a 'step' for a couple of reasons:

  • if I were to make it a job I would have to do 3/4 exact same steps from build
  • it was just faster to try it out

Copy link
Member

@thisisdhaas thisisdhaas May 24, 2022

Choose a reason for hiding this comment

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

From some quick Googling, folks advocate for "keeping jobs simple," but there doesn't seem to be a ton of best-practices out there.

I'm curious if it would get rid of the need to repeat steps if you made Publish a separate job, but used the needs directive to specify that the Publish job depends on the Build job?

If so, that feels cleaner to me.

Copy link
Contributor Author

@progremir progremir May 24, 2022

Choose a reason for hiding this comment

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

I like it

id: publish
if: github.ref == 'refs/heads/main'
uses: JS-DevTools/npm-publish@v1
Copy link
Member

@thisisdhaas thisisdhaas May 23, 2022

Choose a reason for hiding this comment

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

Is this a github-developed action or third party? If not github, could you link to it? (Want to verify that it's well-maintained, popular, etc.)

Copy link
Contributor Author

@progremir progremir May 23, 2022

Choose a reason for hiding this comment

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

https://github.com/marketplace/actions/npm-publish

Copy link
Member

@thisisdhaas thisisdhaas May 24, 2022

Choose a reason for hiding this comment

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

👀 👀 👀 Screen Shot 2022-05-23 at 11 29 33 PM 👀 👀 👀

looks pretty sane to me!

with:
token: ${{ secrets.NPM_TOKEN }}
check-version: true
Copy link
Member

@thisisdhaas thisisdhaas May 23, 2022

Choose a reason for hiding this comment

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

what does 'check-version' do?

Copy link
Contributor Author

@progremir progremir May 23, 2022

Choose a reason for hiding this comment

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

Only publish to NPM if the version number in package.json differs from the latest on NPM

Copy link
Member

@thisisdhaas thisisdhaas May 24, 2022

Choose a reason for hiding this comment

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

should we also use greater-version-only, to be safe?


- name: Version update
if: steps.publish.outputs.type != 'none'
Copy link
Member

@thisisdhaas thisisdhaas May 23, 2022

Choose a reason for hiding this comment

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

is this just to make sure there wasn't an error? (Is it possible that the Publish step could fail but still output something?)

(I'm not very familiar with GH actions, so if there's a reference talking about best practices for this stuff, I'd love to see it!)

Copy link
Contributor Author

@progremir progremir May 23, 2022

Choose a reason for hiding this comment

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

I mostly copied it from an example action.
This one was to see the version update output in the Github actions console.

Copy link
Member

@thisisdhaas thisisdhaas May 24, 2022

Choose a reason for hiding this comment

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

I read the docs and now I get it!

run: |
echo "Version changed: ${{ steps.publish.outputs.old-version }} => ${{ steps.publish.outputs.version }}"
Copy link
Member

@thisisdhaas thisisdhaas May 23, 2022

Choose a reason for hiding this comment

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

how does JS-DevTools/npm-publish choose the new version number? Is it using something like semver? (If so, how do you choose whether to bump the patch/minor/major version?)

Copy link
Contributor Author

@progremir progremir May 23, 2022

Choose a reason for hiding this comment

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

It uses the version we specify in package.json

Copy link
Member

@thisisdhaas thisisdhaas May 24, 2022

Choose a reason for hiding this comment

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

Perfect.