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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate VSCE to use local-dev-lib instead of cli-lib #264

Merged
merged 4 commits into from May 14, 2024
Merged

Conversation

jsines
Copy link
Contributor

@jsines jsines commented May 9, 2024

Hits the couple tasks here: #247
Depends on this PR from local-dev-lib: HubSpot/hubspot-local-dev-lib#134

Diff size is mostly package-lock.json and file removals 馃槃

@jsines jsines requested review from TanyaScales and j-malt May 9, 2024 20:07
@@ -1,5 +1,2 @@
#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

node ./scripts/sanityCheckEnLyaml.js
Copy link
Contributor Author

@jsines jsines May 9, 2024

Choose a reason for hiding this comment

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

A little historical context on these lang changes - with cli-lib there was no bundling process and the lyaml files didn't get shipped with the package, so I had set it up to download the lang data on commit to keep it updated and then we could bundle it like this & initialize the data at run time. No longer necessary with local-dev-lib bundling its own lang data so we don't see those failures

};
category: string;
}

Copy link
Contributor Author

@jsines jsines May 9, 2024

Choose a reason for hiding this comment

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

This type exists in local-dev-lib now, so changing our usage to that one and removing this. Theres some others I'm sure that we could swap to LDL types but this is just one I specifically noted in the migration checklist

env,
});
name
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

updateConfigWithPersonalAccessKey no longer exists in LDL so fetching token first and then using updateConfigWithAccessToken

Copy link
Contributor

@j-malt j-malt left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Member

@TanyaScales TanyaScales left a comment

Choose a reason for hiding this comment

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

LGTM

@jsines jsines merged commit 76191d8 into master May 14, 2024
1 check passed
@jsines jsines deleted the js/migrate-LDL branch May 14, 2024 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants