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

Improve efficiency of yarn-install's registry overrride step #373

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ascrazy
Copy link
Contributor

@ascrazy ascrazy commented Jan 9, 2025

Description

Working with yarn-install I have noticed it's registry override step is quite complex and is not as fast as it could be, so I just asked an LLM to rewrite script to do the same thing but in one pass over yarn.lock and got a working solution that's also kind of more straightforward

Here's performance comparison of existing and new implementation on yarn.lock taken from toptal/client-portal:
image

How to test

Development checks

  • Add changeset according to guidelines (if needed)
  • Remove benchmark code from PR
PR commands

List of available commands:

  • @toptal-anvil ping reviewers - Ping teams for review

Copy link

changeset-bot bot commented Jan 9, 2025

🦋 Changeset detected

Latest commit: 2f22afd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
davinci-github-actions Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ascrazy ascrazy force-pushed the yarn-install-registry-overrride-in-one-pass branch from 29e6af9 to e2b2be4 Compare January 9, 2025 07:35
@ascrazy ascrazy added the no-jira label Jan 9, 2025
@ascrazy ascrazy force-pushed the yarn-install-registry-overrride-in-one-pass branch 5 times, most recently from 9c9a4cf to 7b4d437 Compare January 10, 2025 09:44
@ascrazy ascrazy marked this pull request as ready for review January 10, 2025 09:47
@ascrazy ascrazy requested a review from a team as a code owner January 10, 2025 09:47
Copy link
Contributor

@denieler denieler left a comment

Choose a reason for hiding this comment

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

Nice! thanks for the improvement 🙇‍♂️ I'd only suggest excluding benchmarks from the final committed files 🙇‍♂️

@ascrazy ascrazy requested a review from a team January 10, 2025 12:11
yarn-install/action.yml Outdated Show resolved Hide resolved
@ascrazy ascrazy force-pushed the yarn-install-registry-overrride-in-one-pass branch from 7b4d437 to 2f22afd Compare January 13, 2025 06:44
@ascrazy ascrazy requested a review from pudek357 January 13, 2025 06:45
Copy link
Contributor

@pudek357 pudek357 left a comment

Choose a reason for hiding this comment

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

Nice job, thanks for testing in SP and platform 👍

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.

3 participants