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.

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

remove volta from CI #1594

Merged
merged 6 commits into from
Oct 31, 2023
Merged

remove volta from CI #1594

merged 6 commits into from
Oct 31, 2023

Conversation

mansona
Copy link
Member

@mansona mansona commented Sep 6, 2023

This PR attempts to just use the standard actions/setup-node infra to setup node because the volta infra keeps crashing on us

.npmrc Show resolved Hide resolved
@void-mAlex
Copy link
Collaborator

seems like the one test that is failing is related directly to volta shenanigans ... we may need to see how that translates in a world without volta
https://github.com/void-mAlex/embroider/blob/main/tests/scenarios/shared-internals-test.ts#L44

Copy link
Contributor

@ef4 ef4 left a comment

Choose a reason for hiding this comment

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

If we're going to remove Volta lets do it everywhere, in favor of using pnpm's built-in node version management via npmrc files.

The tests that need a specific node version should emit their own npmrc files, similar to how they emit their own Volta config now.

@ef4
Copy link
Contributor

ef4 commented Sep 26, 2023

Volta maintainers discussing a fix: volta-cli/volta#1523 (comment)

@mansona
Copy link
Member Author

mansona commented Oct 23, 2023

I'm submitting this PR for consideration again. I have implemented the pnpm node version management for the tests that need it but I have not implemented it across the board.

My reasoning for this is that we specifically have tests that use --no-lockfile because we want to track when our CI is failing because of changes in our dependencies. With volta and pnpm's use-node-version implementation you have to pin an exact version of node which does not seem in the spirit of --no-lockfile. This PR also identified the problem that I describe in #1649 because it also didn't pin an exact version of pnpm

I will add this PR to the meeting agenda for discussion 👍

@mansona mansona force-pushed the remove-volta branch 4 times, most recently from 488ac0b to 416fc97 Compare October 25, 2023 16:14
@mansona mansona self-assigned this Oct 25, 2023
@ef4 ef4 merged commit 518ea37 into main Oct 31, 2023
208 checks passed
@ef4 ef4 deleted the remove-volta branch October 31, 2023 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants