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

build(ci): fix build with pnpm v8 #1576

Closed
wants to merge 8 commits into from
Closed

build(ci): fix build with pnpm v8 #1576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
850e47c
build(ci): fix build with pnpm v8
Sep 5, 2023
2ae6b01
build(ci): remove unneeded master branch manual checkout
Sep 5, 2023
9627e03
build(ci): order release workeflow as the validate one
Sep 5, 2023
3392855
build(ci): spell fix
Sep 5, 2023
53c0df1
build(repo): ensure .nvmrc reference a working node version
Sep 5, 2023
c1a56c4
build(ci): fix test run
Sep 5, 2023
f9be6f4
build(ci): revert node 20 support
Sep 6, 2023
47b7a30
Add back node version 20, issue is fixed on 20.6.1.
jerome-benoit Sep 13, 2023
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
11 changes: 5 additions & 6 deletions .github/workflows/node-windows.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,20 @@ jobs:

strategy:
matrix:
node: ['18', '16', '14']
node: ['20', '18', '16']

name: Node v${{ matrix.node }}
steps:
- name: Configure git line-breaks
run: git config --global core.autocrlf false

- name: Checkout Commit
uses: actions/checkout@v1

- name: Checkout Master
run: git branch -f master origin/master
uses: actions/checkout@v4
Copy link
Collaborator

@shellscape shellscape Sep 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

what's the reasoning here? opening PRs on open source projects means explaining changes so that maintainers and onlookers understand your changes.

Copy link
Author

@jerome-benoit jerome-benoit Sep 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

And maintainers of open source project should avoid to push on master partially tested changes breaking the CI :-)

The checkout forcing master branch reset was unneeded since the GH actions has been fixed to properly checkout everything with force-depth 0, even with the v1 major version.

Copy link
Author

@jerome-benoit jerome-benoit Sep 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

A CI run approval is needed to ensure some of the changes are working as intended.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This might be language barrier but I'm not digging the words you're using; they are condescending.

The checkout forcing master branch usage was unneeded since the GH actions has been fixed to properly use the default repo branch, even with the v1 major version.

Please cite your source.

Copy link
Author

Choose a reason for hiding this comment

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

This might be language barrier but I'm not digging the words you're using; they are condescending.

It's just a fact: code partially tested has be been pushed directly on master breaking the CI. It's neither a good practice nor recommended. Facts pointing has nothing to do with condescending.

The checkout forcing master branch usage was unneeded since the GH actions has been fixed to properly use the default repo branch, even with the v1 major version.

Please cite your source.

The force-depth tunable to checkout branches is here since the very first release.

The latest code in that PR is working properly on the forked repo.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm really not digging the vibe of the discourse here. I asked for a cited source, not additional hyperbole. If you'd like to continue the PR, please provide a link to a commit or issue in the actions/checkout repo that we can reference. And I kindly request that your replies not continue to be combative. If that isn't acceptable please consider closing your PR. We don't expect everyone to be chill, but with the little time maintainers have, we're not going to engage with contributors who are not.

Copy link
Author

@jerome-benoit jerome-benoit Sep 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'm really not digging the vibe of the discourse here. I asked for a cited source, not additional hyperbole. If you'd like to continue the PR, please provide a link to a commit or issue in the actions/checkout repo that we can reference.

The force-depth tunable to checkout branches is here since probably minor revision of the version 1 or the version 2. The reference to its introduction has no importance, the rationale on its usage is explained in the description.

  • The PR is working as intended
  • The changes are detailed
  • The regression on PRs validation CI is fixed
  • The project codeflow using PRs will be working again

That's all that PR is meant for.

with:
fetch-depth: 0

- name: Setup Node
uses: actions/setup-node@v1
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node }}

Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/pr-title.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v2
uses: actions/checkout@v4
- name: Check PR Title
uses: clowdhaus/actions/pr-title@v0.1.0
uses: clowdhaus/actions/pr-title@v0.4.0
with:
on-fail-message: "Your PR title doesn't match the required format. The title should be in the conventional commit (https://www.conventionalcommits.org/en/v1.0.0-beta.4/) format. e.g.\n\n```\nchore(plugin-name): add pr title workflow\n```"
title-regex: '^(build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test)(\([\w|,|\-|\|]+\))?(!)?\:\s.*$'
Expand Down
20 changes: 9 additions & 11 deletions .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,34 +18,32 @@ jobs:

steps:
- name: Checkout Commit
uses: actions/checkout@v1
uses: actions/checkout@v4
with:
ref: 'master'
fetch-depth: 0

- name: Setup Node
uses: actions/setup-node@v1
uses: actions/setup-node@v3
with:
node-version: 18
registry-url: https://registry.npmjs.org/

- name: Checkout Master
- name: Install pnpm
run: |
git branch -f master origin/master
git checkout master
npm install pnpm -g;

- name: Sanity Check
run: |
echo branch `git branch --show-current`;
echo node `node -v`;
echo pnpm `pnpm -v`;

- name: Initliaze .npmrc
- name: Initialize .npmrc
run: >
echo -e "//registry.npmjs.org/:_authToken=${{ secrets.NPM_TOKEN }}\n$(cat .npmrc)" > .npmrc
&& cat -n .npmrc

- name: Install pnpm
run: |
npm install pnpm -g;
echo node `pnpm -v`;

- name: Set Git Config
run: |
git config pull.rebase false
Expand Down
13 changes: 6 additions & 7 deletions .github/workflows/validate.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,29 @@ jobs:

strategy:
matrix:
node: ['18', '16', '14']
node: ['18', '16']

name: Node v${{ matrix.node }}

steps:
- name: Checkout Commit
uses: actions/checkout@v1
uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Setup Node
uses: actions/setup-node@v1
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node }}

- name: Checkout Master
Copy link
Collaborator

Choose a reason for hiding this comment

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

and here?

run: git branch -f master origin/master

- name: Install pnpm
run: npm install pnpm -g

- name: Sanity Check
run: |
echo branch `git branch --show-current`;
echo node `node --version`;
echo yarn `pnpm --version`
echo pnpm `pnpm --version`

- name: pnpm install
run: pnpm install
Expand Down
2 changes: 1 addition & 1 deletion .nvmrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
14
16