build(ci): fix build with pnpm v8 #1576
Conversation
- pnpm v8 has dropped support for node 14, remove it - add node 20 to node versions matrix - bump GH actions version to latest Signed-off-by: Jérôme Benoit <[email protected]>
jerome-benoit
force-pushed
the
fix-ci
branch
from
7c31bb3 to
850e47c
Compare
Signed-off-by: Jérôme Benoit <[email protected]>
|
|
||
| - name: Checkout Master | ||
| run: git branch -f master origin/master | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
what's the reasoning here? opening PRs on open source projects means explaining changes so that maintainers and onlookers understand your changes.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
A CI run approval is needed to ensure some of the changes are working as intended.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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/checkoutrepo 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: | ||
| node-version: ${{ matrix.node }} | ||
|
|
||
| - name: Checkout Master |
Signed-off-by: Jérôme Benoit <[email protected]>
Signed-off-by: Jérôme Benoit <[email protected]>
Signed-off-by: Jérôme Benoit <[email protected]>
Signed-off-by: Jérôme Benoit <[email protected]>
jerome-benoit
changed the title
jerome-benoit
changed the title
jerome-benoit
force-pushed
the
fix-ci
branch
from
2dff3da to
c1a56c4
Compare
At least until babel/babel#15927 is fixed Signed-off-by: Jérôme Benoit <[email protected]>
|
Closing in favor of 5469967 |
Repository CI
This PR contains:
Are tests included?
Breaking Changes?
Description
Since 0278630, CI is broken, which hinders the merge of important bug fixes such as #1571.
Changes:
force-depth: 0in the github action to fetch all branches (faster)Changes validation:
The regression root cause is not addressed:
Some references to node 14 usage are left in the code, I do not have the knowledge to handle them
The alternative fix is to revert the culprit commit (dunno why it's not have been done by maintainers on the first CI validation failure)