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

[SG-39284] Upgrade yarn to the latest version #39728

Merged
merged 55 commits into from
Sep 1, 2022

Conversation

gitstart-sourcegraph
Copy link
Collaborator

@gitstart-sourcegraph gitstart-sourcegraph commented Jul 31, 2022

Description

Setup:

  • Follow the official migration guide from yarn

Migrate:

  • Remove yarn-deduplicate package, replace yarn deduplicate with native yarn dudupe
  • Remove flags: —mutex —silient —ignore-script —ignore-engine
  • Replace --cwd package/path with workspace package_name
  • Append yarn in front of binary script in scripts config, ex. cross-env -> yarn cross-env
  • Fix some TS build errors
  • CI: Remove cache client/extension-api/node_modules
  • CI: Add cache .yarn/cache

Refs

Sourcegraph Issue
Gitstart Ticket

Test plan

Ensure that CI checks are green.

App preview:

Check out the client app preview documentation to learn more.

@gitstart-sourcegraph gitstart-sourcegraph added gitstart Contract partner frontend-platform Issues related to our frontend platform, owned collectively by our frontend crew. labels Jul 31, 2022
@cla-bot cla-bot bot added the cla-signed label Jul 31, 2022
@gitstart-sourcegraph gitstart-sourcegraph self-assigned this Jul 31, 2022
@gitstart-sourcegraph gitstart-sourcegraph force-pushed the contractors/SG-39284.base branch 2 times, most recently from 8b4ac1c to 90f0c27 Compare August 1, 2022 05:37
Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

@gitstart-sourcegraph, please force-push to remove the latest commit from the git history of our the repository. Removing all zip archives committed to the repo is important to avoid polluting git history with large files.

We do not want to use the Plug'n'Play feature with zero installs. It's mentioned in the issue description. Initially, we want to use the node-modules nodeLinker to minimize the changes required for the upgrade.

We want to upgrade to the latest version of yarn with minimal changes required in our codebase. Mainly it should be commands and some configuration files. Let me know if you bump into any sweeping changes while working on the task.

@gitstart
Copy link
Contributor

gitstart commented Aug 1, 2022

:D thanks @valerybugakov actually we are going to ask you about it too

@gitstart-sourcegraph gitstart-sourcegraph changed the title [SG-39284] Upgrade yarn to the latest version - Setup [SG-39284] Upgrade yarn to the latest version Aug 1, 2022
Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

Let's try removing web-ext package, which is not used in the repo anymore, to remove transitive dependency on the dtrace-provider package, which causes failure here.

Also, let's explicitly add @sentry/cli to our package.json to see if it resolves another issue reported by the build linked above.

@gitstart-sourcegraph
Copy link
Collaborator Author

As discussed on Slack, we may need to upgrade Yarn on code-intel-extensions. Still wait for the confirmation from @valerybugakov

Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

The current state looks great! I'm still testing different commands locally and asked a couple more people to take a look. Let's merge it tomorrow if nothing comes up.

@eseliger
Copy link
Member

no longer seeing the issue with sg start 👍

"test:regression": "yarn task:mocha './src/regression/**/*.test.ts' --exit",
"test": "yarn run -T jest --testPathIgnorePatterns end-to-end --testPathIgnorePatterns regression integration",
"task:mocha": "yarn run -T cross-env TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' mocha",
"test:regression": "yarn run -T task:mocha './src/regression/**/*.test.ts' --exit",
Copy link
Member

Choose a reason for hiding this comment

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

Might the -T flag here be the reason for this CI failure?

@@ -224,18 +224,18 @@
"prewatch": "yarn build-inline-extensions",
"vscode:prepublish": "yarn build-inline-extensions && yarn build",
"build-inline-extensions": "node scripts/build-inline-extensions",
"task:gulp": "cross-env NODE_OPTIONS=\"--max_old_space_size=8192\" gulp",
"task:gulp": "yarn cross-env NODE_OPTIONS=\"--max_old_space_size=8192\" gulp",
Copy link
Member

Choose a reason for hiding this comment

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

Should we use the -T flag here in the same manner we do it for the task:gulp NPM script in the web package?

Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

Pushed a couple of fixes and triggered the main-dry-run build.

Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed frontend-platform Issues related to our frontend platform, owned collectively by our frontend crew. gitstart Contract partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade yarn to the latest version
7 participants