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

Jump to bottom

NPM/Yarn examples: Use lock file usually #26699

Merged
merged 7 commits into from
Jul 21, 2023
Merged

NPM/Yarn examples: Use lock file usually #26699

merged 7 commits into from
Jul 21, 2023

Conversation

SchulteMarkus
Copy link
Contributor

@SchulteMarkus SchulteMarkus commented Jul 14, 2023 •
edited
Loading

Why:

  • Highlighting to use package-lock.json / yarn.lock for installing usually
  • Replacing --frozen-lockfile with its successor

What's being changed:

  • Example using npm
  • Example using Yarn

Check off the following:

  • I have reviewed my changes in staging, available via the View deployment link in this PR's timeline.

    • For content changes, you will also see an automatically generated comment with links directly to pages you've modified. The comment won't appear if your PR only edits files in the data directory.

  • For content changes, I have completed the self-review checklist.

@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Jul 14, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 14, 2023 •
edited
Loading

Automatically generated comment ℹ️

This comment is automatically generated and will be overwritten every time changes are committed to this branch.

The table contains an overview of files in the content directory that have been changed in this pull request. It's provided to make it easy to review your changes on the staging site. Please note that changes to the data directory will not show up in this table.

Content directory changes

You may find it useful to copy this table into the pull request summary. There you can edit it to share links to important articles or changes and to give a high-level overview of how the changes in your pull request support the overall goals of the pull request.

Source Preview Production What Changed
actions/automating-builds-and-tests/building-and-testing-nodejs.md fpt
ghec
ghes@ 3.9 3.8 3.7 3.6
ghae
 fpt
ghec
ghes@ 3.9 3.8 3.7 3.6
ghae

fpt: Free, Pro, Team
ghec: GitHub Enterprise Cloud
ghes: GitHub Enterprise Server
ghae: GitHub AE

@cmwilson21
Copy link
Contributor

@SchulteMarkus Thanks so much for submitting a PR! I'll get this triaged for review ⚡

@cmwilson21 cmwilson21 added content This issue or pull request belongs to the Docs Content team actions This issue or pull request should be reviewed by the docs actions team waiting for review Issue/PR is waiting for a writer's review and removed triage Do not begin working on this issue until triaged by the team labels Jul 14, 2023
@hubwriter hubwriter self-requested a review July 19, 2023 09:17
hubwriter
hubwriter reviewed Jul 19, 2023
View reviewed changes
Copy link
Contributor

@hubwriter hubwriter left a comment

Choose a reason for hiding this comment

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

@SchulteMarkus - Thank you for submitting this PR.

I've asked someone from our Actions dev team to take a look at this.

hubwriter
hubwriter requested changes Jul 20, 2023
View reviewed changes
Copy link
Contributor

@hubwriter hubwriter left a comment

Choose a reason for hiding this comment

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

@SchulteMarkus - Thanks again for this PR.

One of the Actions developers has taken a look at this and has approved the first part of the changes but said of the changes in the "Example using Yarn" section that the changes use Yarn 2 commands whereas, out of the box, GitHub hosted runners ships with Yarn 1.22.19 (https://github.com/actions/runner-images/blob/main/images/linux/Ubuntu2204-Readme.md).

To avoid problems if people haven't modified this to upgrade the Yarn version on a runner, we need to be sure that the changes you are proposing will work using Yarn 1.22.19.

Could you check this and confirm here?

content/actions/automating-builds-and-tests/building-and-testing-nodejs.md Show resolved Hide resolved
@SchulteMarkus SchulteMarkus changed the title Yarn update frozen lockfile NPM/Yarn examples: Use lock file usually Jul 21, 2023
@SchulteMarkus
Copy link
Contributor Author

One of the Actions developers has taken a look at this and has approved the first part of the changes but said of the changes in the "Example using Yarn" section that the changes use Yarn 2 commands whereas, out of the box, GitHub hosted runners ships with Yarn 1.22.19 (https://github.com/actions/runner-images/blob/main/images/linux/Ubuntu2204-Readme.md).

To avoid problems if people haven't modified this to upgrade the Yarn version on a runner, we need to be sure that the changes you are proposing will work using Yarn 1.22.19.

Could you check this and confirm here?

Ui, good that you asked!
I have added one more commit, so that the Yarn command is again Yarn 1.x compatible. I have updated the PR itself, too.
This PR changed a bit from it's indentation (not using yarn --frozen-lockfile any more) to now (using lock files as first examples). In my opinion, it's still an improvement.

@SchulteMarkus SchulteMarkus requested a review from hubwriter July 21, 2023 10:37
@SchulteMarkus
Copy link
Contributor Author

SchulteMarkus commented Jul 21, 2023 •
edited
Loading

Failure https://github.com/github/docs/actions/runs/5621341748/job/15231953003?pr=26699 seems to be GitHub infrastructure related (status 502). @hubwriter, can you execute the build again?

SchulteMarkus and others added 7 commits July 21, 2023 12:41
@SchulteMarkus
Using yarn.lock as first Yarn example
66b1777 
Using the yarn.lock file should be the default in build pipelines. To emphasise this, this example comes first.
@SchulteMarkus
Replace the --frozen-lockfile parameter with its successor
ad48f44 
--frozen-lockfile is legacy[1]. 
Using the parameters "--immutable --immutable-cache --check-cache"is the replacement for the previous function according to https://yarnpkg.com/cli/install#examples

[1] "For backward compatibility we offer an alias under the name of --frozen-lockfile, but it will be removed in a later release." https://yarnpkg.com/cli/install
@SchulteMarkus
Uniform representation of 'yarn.lock'
9990a88
@SchulteMarkus
Fixed tiny typo at 'Example using Yarn'
be3bdb1
@SchulteMarkus
Using 'npm ci' as first npm example
7101ffe 
Using 'npm ci' should be the default in npm build pipelines. To emphasise this, this example comes first.
@SchulteMarkus
Use --frozen-lockfile at 'Example using Yarn'
bab9983 
As GitHub hosted runners are serving Yarn 1.x[1], going back to --frozen-lockfile
"If you need reproducible dependencies, which is usually the case with the continuous integration systems, you should pass --frozen-lockfile flag." https://classic.yarnpkg.com/en/docs/cli/install#toc-yarn-install

[1] #26699 (review)
@hubwriter
Merge branch 'main' into yarn-update-frozen-lockfile
53ac40a
hubwriter
hubwriter approved these changes Jul 21, 2023
View reviewed changes
Copy link
Contributor

@hubwriter hubwriter left a comment

Choose a reason for hiding this comment

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

@SchulteMarkus - Many thanks for the changes.

I'll get this merged/published now. Thank you for helping to keep the docs up to date. Much appreciated.

@hubwriter hubwriter added this pull request to the merge queue Jul 21, 2023
Merged via the queue into github:main with commit 0a11854 Jul 21, 2023
@github-actions
Copy link
Contributor

Thanks very much for contributing! Your pull request has been merged 🎉 You should see your changes appear on the site in approximately 24 hours. If you're looking for your next contribution, check out our help wanted issues ⚡

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hubwriter hubwriter hubwriter approved these changes

Assignees
No one assigned
Labels
actions This issue or pull request should be reviewed by the docs actions team content This issue or pull request belongs to the Docs Content team waiting for review Issue/PR is waiting for a writer's review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SchulteMarkus @cmwilson21 @hubwriter