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

Enhance CI/CD workflow with Yarn 3, Node.js 14, and GitHub Actions #799

Merged
merged 25 commits into from Jan 22, 2023

Conversation

emcniece
Copy link
Contributor

Generates an npm-shrinkwrap.json file during releases, adds it to Appveyor artifacts where it will live for 1 month post-CI-run, and hopefully deploys it to Github releases.

We'll test this in an upcoming tag where we include #796.

Resolves #794.

@emcniece emcniece requested a review from cheton January 17, 2023 04:23
@cheton
Copy link
Collaborator

cheton commented Jan 17, 2023

I suggest including the lock files 'package-lock.json' or 'yarn.lock' in the repository by removing them from the '.gitignore' file. What are your thoughts on this?

@emcniece
Copy link
Contributor Author

Including package-lock.json is a good idea, yes! It is intended to be committed to VCS. I am unsure about the variance in development platforms however.

During the CI runs for this PR I extracted npm-shrinkwrap.json and performed a diff between Linux, Mac, and Windows builds. There are many changes between each platform, mostly package signatures such as:

<       "resolved": "https://registry.npmjs.org/fd-slicer/-/fd-slicer-1.1.0.tgz",
<       "integrity": "sha512-cE1qsB/VwyQozZ+q1dGxR8LBYNZeofhEdUNGSMbQD3Gw2lAzX9Zb3uIU6Ebc/Fmyjo9AWWfnn0AUCHqtevs/8g==",
31144,31145d26064
<       "resolved": "https://registry.npmjs.org/yeast/-/yeast-0.1.2.tgz",
<       "integrity": "sha512-8HFIh676uyGYP6wP13R/j6OJ/1HwJ46snpvzE7aHAN3Ryqh2yX6Xox2B4CUmTwwOIzlG3Bs7ocsP5dZH/R1Qbg==",
31150,31151d26068
<       "resolved": "https://registry.npmjs.org/yn/-/yn-3.1.1.tgz",
<       "integrity": "sha512-Ux4ygGWsu2c7isFWe8Yu1YluJmqVhxqK2cLXNQA5AcC3QfbGNpM7fu0Y8b/z16pXLnFxZYvWhd3fhBY9DLmC6Q==",

There are however also specific package differences (I assume for the included binaries) such as:

>     "node_modules/watchpack/node_modules/fsevents": {
>       "version": "2.3.2",
>       "dev": true,
>       "license": "MIT",
>       "optional": true,
>       "os": [
>         "darwin"
>       ],
>       "engines": {
>         "node": "^8.16.0 || ^10.6.0 || >=11.0.0"
>       }
>     },

This leads me to think that package-lock.json will differ between OS builds, and since the community here has contributors from many platforms we could see frequent changes to package-lock.json that revert previous changes.

Perhaps this won't cause any problems. If it does become an issue, we could explore a unified development environment in a Docker container or something.

This PR currently provides evidence of package versions in a given CI run, which satisfies the original ask. I'm ok with either option, adding package-lock.json or continuing to ignore it. Your preference?

@colin-campbell
Copy link
Contributor

The first question should maybe be npm or yarn? It shouldn't really be npm for development and yarn for CI.
If yarn is used, shrinkwrap is kinda redundant. In fact running npm ci or yarn --frozen-lock-file from a tagged version gives you a canonical install without shrinkwrap at all.

@parachvte
Copy link

Reading through the npm shrinkwrap doc.

When the library is a global commend line tool / dev dependency, you will want to lock down dep versions to make it stable as possible. In case of cncjs, it's a desktop application, in most cases users won't need the published npm-shrinkwrap.json. package-lock.json or yarn.lock is more useful for developers.

@cheton
Copy link
Collaborator

cheton commented Jan 17, 2023

I intend to use Yarn as the package manager, as I am familiar with it and have used it extensively on recent projects."

I will perform a test in a separate branch (feature/next) for both Yarn and Node.js upgrade. If everything works as expected, I will update the pull request with the change.

@cheton cheton changed the title Publish npm-shrinkwrap.json file to Appveyor artifacts ci: upgrade to Yarn 3 and Node.js 14 Jan 18, 2023
@cheton cheton changed the title ci: upgrade to Yarn 3 and Node.js 14 Enhance CI workflow by upgrading to Yarn 3 and Node.js 14 Jan 18, 2023
@cheton
Copy link
Collaborator

cheton commented Jan 18, 2023

Both Yarn 3 and Node.js 14 have been successfully upgraded. The build results can be viewed at the following link: https://ci.appveyor.com/project/cheton/cncjs/history

In addition, I plan to create a new pull request to migrate the CI pipeline from AppVeyor to GitHub actions for further efficiency.

@cheton cheton changed the title Enhance CI workflow by upgrading to Yarn 3 and Node.js 14 Enhance CI/CD workflow with Yarn 3, Node.js 14, and GitHub Actions Jan 19, 2023
@emcniece
Copy link
Contributor Author

Impressive work @cheton 🚀

@cheton cheton merged commit a0ccc16 into cncjs:master Jan 22, 2023
@cheton
Copy link
Collaborator

cheton commented Jan 22, 2023

Published v1.10.0 🚀

image

What's Changed

Highlight

This release includes the following upgrades and changes:

  • Electron has been upgraded from v4 to v22
  • Serialport has been upgraded from v9 to v10
  • The CI system has been changed from using AppVeyor and CircleCI to using GitHub Actions and CircleCI
  • Node.js has been upgraded from v12 to v14
  • The package manager has been changed from NPM to Yarn

New Contributors

Full Changelog: v1.9.28...v1.10.0

@emcniece emcniece deleted the fix/publish-package-lock branch January 23, 2023 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide upstream curated package-lock.json
4 participants