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.

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

Jump to bottom

ci: publish workflow #2591

Closed
wants to merge 6 commits into from
Closed

ci: publish workflow #2591

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
64618fd
ci: publish workflow
Shinigami92 Dec 28, 2023
a1c8143
Merge branch 'next' into publish-via-ci
Shinigami92 Jun 21, 2024
84b4dfe
Update .github/workflows/publish.yml
Shinigami92 Jun 21, 2024
282a624
configure permissions
Shinigami92 Jun 21, 2024
66e6179
chore: set next dist-tag to current package version
Shinigami92 Jun 24, 2024
0191f87
Merge branch 'next' into publish-via-ci
Shinigami92 Jun 24, 2024
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
44 changes: 44 additions & 0 deletions .github/workflows/publish.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
name: Publish

on:
workflow_dispatch:

permissions:
contents: read # to fetch code (actions/checkout)

jobs:
Shinigami92 marked this conversation as resolved.
Show resolved Hide resolved
release:
runs-on: ubuntu-latest
name: Release
steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Install pnpm
uses: pnpm/action-setup@v2

- name: Set node version to 20
uses: actions/setup-node@v4
with:
node-version: 20
cache: 'pnpm'

- name: Prepare
run: pnpm install --frozen-lockfile

- name: Set publishing config
run: pnpm config set '//registry.npmjs.org/:_authToken' "${NPM_AUTH_TOKEN}"
env:
NPM_AUTH_TOKEN: ${{ secrets.NPM_AUTH_TOKEN }}

- name: Publish
Copy link
Member

Choose a reason for hiding this comment

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

According to our release checklist, we should remove the "next branch warning" before the publish.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what this has to do with this PR 🤔

Copy link
Member

@ST-DDT ST-DDT Jun 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

confused: IMO This PR is intended to partially automate the release process, so it should actually contain the same steps as the manual workflow?!
If we don't, we have to permanently remove that section from the readme.

Copy link
Member Author

Choose a reason for hiding this comment

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

wait, we ever did remove that banner for a release?! I was not aware of that
if I e.g. look in https://www.npmjs.com/package/@faker-js/faker/v/8.4.1 or https://www.npmjs.com/package/@faker-js/faker/v/9.0.0-alpha.0 we did not do that
Only in the newest release and I was not really part of that

IMO we should just remove that banner completely (in a separate PR using docs: *)
But if you want to you can also leave this open until thursday

Copy link
Member

Choose a reason for hiding this comment

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

FFR: The comment was added for this issue:

Copy link
Member Author

Choose a reason for hiding this comment

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

After a not so good sleep last night, I decided (at least for my self) that I'm not interested in manipulating repository code from a workflow and so also dont like to work further on that topic especially not from this PR, as the scope and intend was something different from my side.
I just wanted to provide a workflow that can be used to release the "current" state of next branch without having to use credentials manually.

So feel free to either provide a follow-up PR (my preference) or jump into this PR and take it over (not my preference).

run: |
PACKAGE_DIST_TAG=$(node -e "console.log(/^\d+\.\d+\.\d+(\-(\w+)\.\d+)$/.exec(require('./package.json').version)?.[2] || 'latest')")
pnpm publish --access public --tag $PACKAGE_DIST_TAG
Shinigami92 marked this conversation as resolved.
Show resolved Hide resolved

- name: Set next dist-tag
run: |
PACKAGE_VERSION=$(node -e "console.log(require('./package.json').version)")
pnpm dist-tag add @faker-js/faker@$PACKAGE_VERSION next
Comment on lines +38 to +44
Copy link
Member

Choose a reason for hiding this comment

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

IMO we should tag it with next by default and set the latest only when it is a stable release:

Suggested change
PACKAGE_DIST_TAG=$(node -e "console.log(/^\d+\.\d+\.\d+(\-(\w+)\.\d+)$/.exec(require('./package.json').version)?.[2] || 'latest')")
pnpm publish --access public --tag $PACKAGE_DIST_TAG
- name: Set next dist-tag
run: |
PACKAGE_VERSION=$(node -e "console.log(require('./package.json').version)")
pnpm dist-tag add @faker-js/faker@$PACKAGE_VERSION next
PACKAGE_VERSION=$(node -e "console.log(require('./package.json').version)")
pnpm publish --access public --tag next
if [[ "$PACKAGE_VERSION" != *"-"* ]]; then
pnpm dist-tag add @faker-js/faker@$PACKAGE_VERSION latest
fi

Not tested.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, we are extracting the dist-tag out of the version field and then e.g. alpha, beta or whatever is used
This is default tagging behavior guided to semver

Copy link
Member

@ST-DDT ST-DDT Jun 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

confused: But we don't have alpha or beta tags!? Or did we remove them after stable release?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh good that we found out that we misused that before 🫤

just some examples that use e.g. beta dist-tags (which are not my own packages)