fix: pin bahmutov/npm-install version #3587
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
|
Hmm maybe we should just remove this step altogether and instead call uses: actions/setup-node@v3
with:
node-version-file: ".nvmrc"
cache: "yarn"@kentcdodds I see |
|
Yeah based on my testing, |
jenseng
added a commit
to jenseng/remix
that referenced
this pull request
Jun 28, 2022
We already use `actions/setup-node@v3` which has equivalent caching functionality that isn't broken for yarn+windows (or yarn 2+ across the board). So just use that wherever possible. In addition to fixing builds, this has the added benefit of slightly speeding them up, since we won't double-cache things (in some cases we had both caches enabled). For context, see bahmutov/npm-install#146, in particular [this comment](bahmutov/npm-install#146 (comment)) and linked test output. This is an alternative to remix-run#3587 or otherwise waiting for `bahmutov/npm-install` to get fixed.
jenseng
added a commit
to jenseng/remix
that referenced
this pull request
Jun 28, 2022
We already use `actions/setup-node@v3` which has equivalent caching functionality that isn't broken for yarn+windows (or yarn 2+ across the board). So just use that wherever possible. In addition to fixing builds, this has the added benefit of slightly speeding them up, since we won't double-cache things (in some cases we had both caches enabled). Making this PR against `main` instead `dev`, since the jobs explicitly use `remix-run/remix/.github/workflows/reusable-test.yml@main`, but I can switch it up if y'all like. Just trying to get my 4-month-old PR unblocked yet again 😆😭 For context, see bahmutov/npm-install#146, in particular [this comment](bahmutov/npm-install#146 (comment)) and linked test output. This is an alternative to remix-run#3587 or otherwise waiting for `bahmutov/npm-install` to get fixed.
jenseng
added a commit
to jenseng/remix
that referenced
this pull request
Jun 28, 2022
We already use `actions/setup-node@v3` which has equivalent caching functionality that isn't broken for yarn+windows (or yarn 2+ across the board). So just use that wherever possible. In addition to fixing builds, this has the added benefit of slightly speeding them up, since we won't double-cache things (in some cases we had both caches enabled). Making this PR against `main` instead `dev`, since the jobs explicitly use `remix-run/remix/.github/workflows/reusable-test.yml@main`, but I can switch it up if y'all like. Just trying to get my 4-month-old PR unblocked yet again 😆😭 For context, see bahmutov/npm-install#146, in particular [this comment](bahmutov/npm-install#146 (comment)) and linked test output. This is an alternative to remix-run#3587 or otherwise waiting for `bahmutov/npm-install` to get fixed.
jenseng
force-pushed
the
pin-npm-install-workflow-step
branch
from
37abb25
to
9716a58
Compare
|
Alternative PR, just remove it altogether where we can: #3593 |
jenseng
added a commit
to jenseng/remix
that referenced
this pull request
Jun 28, 2022
We already use `actions/setup-node@v3` which has equivalent caching functionality that isn't broken for yarn+windows (or yarn 2+ across the board). So just use that wherever possible. In addition to fixing builds, this has the added benefit of slightly speeding them up, since we won't double-cache things (in some cases we had both caches enabled). Making this PR against `main` instead `dev`, since the jobs explicitly use `remix-run/remix/.github/workflows/reusable-test.yml@main`, but I can switch it up if y'all like. Just trying to get my 4-month-old PR unblocked yet again 😆😭 For context, see bahmutov/npm-install#146, in particular [this comment](bahmutov/npm-install#146 (comment)) and linked test output. This is an alternative to remix-run#3587 or otherwise waiting for `bahmutov/npm-install` to get fixed.
jenseng
force-pushed
the
pin-npm-install-workflow-step
branch
from
9716a58
to
f031158
Compare
jenseng
commented
Jun 28, 2022
.github/workflows/test.yml
Outdated
| @@ -17,6 +17,6 @@ on: | |||
| jobs: | |||
| test: | |||
| if: github.repository == 'remix-run/remix' | |||
| uses: remix-run/remix/.github/workflows/reusable-test.yml@main | |||
| uses: jenseng/remix/.github/workflows/reusable-test.yml@pin-npm-install-workflow-step | |||
There was a problem hiding this comment.
converted to a draft due to this testing chicken/egg problem ... temporarily setting this so the change actually gets tested :)
There was a problem hiding this comment.
tests passed using this workflow definition: https://github.com/remix-run/remix/actions/runs/2579553824, i've gone ahead and removed this tweak
jenseng
added a commit
to jenseng/remix
that referenced
this pull request
Jun 28, 2022
We already use `actions/setup-node@v3` which has equivalent caching functionality that isn't broken for yarn+windows (or yarn 2+ across the board). So just use that wherever possible. In addition to fixing builds, this has the added benefit of slightly speeding them up, since we won't double-cache things (in some cases we had both caches enabled). Making this PR against `main` instead `dev`, since the jobs explicitly use `remix-run/remix/.github/workflows/reusable-test.yml@main`, but I can switch it up if y'all like. Just trying to get my 4-month-old PR unblocked yet again 😆😭 For context, see bahmutov/npm-install#146, in particular [this comment](bahmutov/npm-install#146 (comment)) and linked test output. This is an alternative to remix-run#3587 or otherwise waiting for `bahmutov/npm-install` to get fixed.
jenseng
added a commit
to jenseng/remix
that referenced
this pull request
Jun 28, 2022
We already use `actions/setup-node@v3` which has equivalent caching functionality that isn't broken for yarn+windows (or yarn 2+ across the board). So just use that wherever possible. In addition to fixing builds, this has the added benefit of slightly speeding them up, since we won't double-cache things (in some cases we had both caches enabled). Making this PR against `main` instead `dev`, since the jobs explicitly use `remix-run/remix/.github/workflows/reusable-test.yml@main`, but I can switch it up if y'all like. Just trying to get my 4-month-old PR unblocked yet again 😆😭 For context, see bahmutov/npm-install#146, in particular [this comment](bahmutov/npm-install#146 (comment)) and linked test output. This is an alternative to remix-run#3587 or otherwise waiting for `bahmutov/npm-install` to get fixed.
v1.8.16 breaks cache miss installs on windows, which blocks any PRs/commits that change dependencies. Opened bahmutov/npm-install#146 to address the underlying issue, but in the meantime we can pin to the prior version to get things working again. Additional context: - Discord: https://discord.com/channels/770287896669978684/940264701785423992/991085961972707338 - Initial PR affected: remix-run#2027 - Repro/Debug PR: remix-run#3584
jenseng
force-pushed
the
pin-npm-install-workflow-step
branch
from
f031158
to
fcea942
Compare
jenseng
added a commit
to jenseng/remix
that referenced
this pull request
Jun 28, 2022
We already use `actions/setup-node@v3` which has equivalent caching functionality that isn't broken for yarn+windows (or yarn 2+ across the board). So just use that wherever possible. In addition to fixing builds, this has the added benefit of slightly speeding them up, since we won't double-cache things (in some cases we had both caches enabled). Making this PR against `main` instead `dev`, since the jobs explicitly use `remix-run/remix/.github/workflows/reusable-test.yml@main`, but I can switch it up if y'all like. Just trying to get my 4-month-old PR unblocked yet again 😆😭 For context, see bahmutov/npm-install#146, in particular [this comment](bahmutov/npm-install#146 (comment)) and linked test output. This is an alternative to remix-run#3587 or otherwise waiting for `bahmutov/npm-install` to get fixed.
|
Closing in favor of #3593 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
v1.8.16 breaks cache miss installs on windows, which blocks any PRs/commits that change dependencies. Opened this issue to address the underlying bug, but in the meantime we can pin to the prior version to get things working again.
Making this PR against
maininsteaddev, since the jobs explicitly useremix-run/remix/.github/workflows/reusable-test.yml@main, but I can switch it up if y'all like. Just trying to get my 4-month-old PR unblocked yet again 😆😭Additional context:
Testing Strategy: