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

Change yarn --frozen-lockfile to --immutable #611

Closed
wants to merge 1 commit into from

Conversation

brandonchinn178
Copy link

Related: #591

Yarn 2 has been out for about two years now, which deprecated this flag. The action should use the updated flag by default, with users of Yarn 1 using install-command to use the old flag

@CLAassistant
Copy link

CLAassistant commented Oct 8, 2022

CLA assistant check
All committers have signed the CLA.

@MikeMcC399
Copy link
Collaborator

@brandonchinn178

I was interested to find your PR which you submitted last year. You are right that some changes need to be made for Yarn 2. As you wrote, Yarn 1 (classic) users would need to change how they use the action, so your suggestion to change the default yarn command from yarn --frozen-lockfile to yarn --immutable would be a breaking change. A breaking change means releasing a new major version of the action, which I expect would delay this change happening.

The ideal situation would be if cypress-io/github-action (GHA) could automatically recognize whether it was dealing with a yarn 1 (classic) or yarn berry (v2 and later) project and then use the appropriate command. I don't know if that can be done reliably or not.

Regarding the details of your suggestion, I'm not sure that it would be necessary to specify --immutable option since the (GHA) runs as CI and therefore it would be set to true by default.

According to the Yarn CLI documentation:

"If the --immutable option is set (defaults to true on CI), Yarn will abort with an error exit code if the lockfile was to be modified (other paths can be added using the immutablePatterns configuration setting). For backward compatibility we offer an alias under the name of --frozen-lockfile, but it will be removed in a later release."

Since the current option --frozen-lockfile is deprecated, some action is necessary in any case so I am planning to follow up on this point and I have opened issue #798 to start this process. Stay tuned!


If you want to test your change you would need to carry out some extra steps:

first rebase on the current master branch, then execute

npm run format
npm run build

and commit the changes to GitHub.

@MikeMcC399
Copy link
Collaborator

@brandonchinn178

Your PR branch is now 172 commits behind the master branch, so it is quite out of date. Are you still interested in this? If yes, and you don't have time at the moment, you could change the status draft. If you don't want to pursue the topic any more, then you could close the PR.

@brandonchinn178
Copy link
Author

Yes, sorry, I missed your comment from three weeks ago. I can update sometime next week.

I knew that --immutable is implied in CI, but I do think being explicit is better. But it's only a weak preference, and I can remove it if you want.

@MikeMcC399
Copy link
Collaborator

@brandonchinn178

It's good to hear you are still active! This PR isn't assigned to anybody in the Cypress team so far, so we haven't had any comment back from them yet and it would be good to hear their opinion before you put any significant effort into changes.

@MikeMcC399
Copy link
Collaborator

@brandonchinn178

Here are some additional comments and suggestions:

Considerations

It would be a breaking change to replace the default installation command yarn --frozen-lockfile by yarn --immutable, and Yarn 1 (Classic) is still in significant productive use. Yarn 1 (Classic) is used by:

If the action's default installation command remains as yarn --frozen-lockfile:

  • Yarn Classic repositories need no special handling. This is the default.
  • A deprecation warning is issued (YN0050) if the action is used on Yarn Modern repositories taking the default installation command. Adding the parameter install-command: yarn install, for instance, when calling the action, adapts to the correct Yarn Modern installation command. This is now documented in the README file.

If the action's default installation command is changed to yarn --immutable:

  • Yarn Classic repositories would experience a breaking change. To preserve the existing functionality they would need to add the parameter install-command: yarn --frozen-lockfile when calling the action.
  • Yarn Modern repositories would experience the deprecation warning being droppped if they had not previously adapted by using install-command: yarn install. Otherwise there is no impact. There would only be a impact at a later time if the deprecation warning matures into a removal of the --frozen-lockfile alias from Yarn Modern. So far, Yarn Modern (Canary 4.0.0-rc.40) still accepts this alias.

Suggestion

So, my suggestion is to delay this PR until either:

  • Yarn Classic is no longer supported or no longer in significant use
  • Yarn Modern removes the alias --frozen-lockfile from the CLI

It would be great to hear from other Yarn users if this makes sense.

@MikeMcC399
Copy link
Collaborator

MikeMcC399 commented May 1, 2023

Does anybody have any further feedback on this PR?

I suggest to close it, since at this time I cannot see that Yarn Classic can be abandoned, and I believe there should continue to be backwards compatibility with Yarn Classic.

The contents of the PR itself are quite simple (one line code change in the action), so it would be easy to re-activate it if necessary.

The usage of github-action together with Yarn Modern is now well documented, with an example, in the README > Yarn Modern section. One additional workflow line of code is necessary to override the deprecated --frozen-lockfile parameter

install-command: yarn install

when using Yarn Modern (and only to avoid a deprecation warning, otherwise it will currently work with no extra measures needed). The additional one line of workflow code does not seem to be too burdensome for users to add and to maintain.

@nagash77
Copy link
Contributor

nagash77 commented May 2, 2023

Yes at this point we have no plans to deprecate Yarn 1. I am going to close this PR for the time being and we can revisit when/if we look to update to support Yarn Modern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants