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

Wr/esm #633

Merged
merged 9 commits into from
Nov 2, 2023
Merged

Wr/esm #633

merged 9 commits into from
Nov 2, 2023

Conversation

WillieRuemmele
Copy link
Contributor

What does this PR do?

ESM

What issues does this PR fix or reference?

@WillieRuemmele WillieRuemmele added enhancement New feature or request USER STORY labels Oct 26, 2023
@git2gus
Copy link

git2gus bot commented Oct 26, 2023

This issue has been linked to a new work item: W-14374934

@@ -44,3 +44,7 @@ node_modules
.idea

oclif.manifest.json

oclif.lock
Copy link
Contributor

Choose a reason for hiding this comment

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

only need 1

"/oclif.manifest.json"
"/oclif.manifest.json",
"/oclif.lock",
"/oclif.lock"
Copy link
Contributor

Choose a reason for hiding this comment

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

only one

Copy link
Contributor

Choose a reason for hiding this comment

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

related to oclif.lock - the postpack and clean:lib scripts should remove the oclif.lock too... Probably should add that to our migrate command


try {
if (filepath.endsWith('node')) {
// This checks if the filepath is executable on Mac or Linux, if it is not it errors.
// This checks if the filepath is executable on Mac or Linux, if it is not it errors.
Copy link
Contributor

Choose a reason for hiding this comment

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

the next 4 changes looks like accidentally added spaces

expect(accessSyncStub).to.have.been.calledOnce;
expect(existsSyncStub).to.have.been.calledTwice;
expect(osTypeStub).to.have.been.calledOnce;
expect(realpathSyncStub).to.have.been.calledOnce;
// expect(realpathSyncStub).to.have.been.calledOnce;
Copy link
Contributor

Choose a reason for hiding this comment

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

Were these a TODO, do the tests not work, has the logic changed, or do these need to be removed?

@mshanemc
Copy link
Contributor

mshanemc commented Nov 2, 2023

QA notes:

[made a commit with the dev-scripts bump to reduce devDeps and bumped sfdx-core]

./bin/dev.js plugins trust verify -n @salesforce/plugin-org
✅ shows that it is signed

./bin/dev.js plugins trust verify -n shane-sfdx-plugins
✅ shows not signed

[link into sf]
✅ installs
📓 note the double ESM warning
📓 also note that we're pointing the "open an issue about the warnings" at ourselves, too. 😄

➜  plugin-trust git:(wr/esm) sf plugins install @salesforce/plugin-org
 ›   Warning: @salesforce/plugin-trust is a linked ESM module and cannot be auto-transpiled. Existing compiled source will be used instead.
 ›   Warning: @salesforce/plugin-trust is a linked ESM module and cannot be auto-transpiled. Existing compiled source will be used instead.
Polling for new version(s) to become available on npm... done
Successfully validated digital signature for @salesforce/plugin-org.
Finished digital signature check.

sf plugins install shane-sfdx-plugins
✅ prompts for verification
warnings and errors still return fine.

add plugin to allowList, reinstall
The plugin [shane-sfdx-plugins] is not digitally signed but it is allow-listed.

@mshanemc mshanemc merged commit cb32533 into main Nov 2, 2023
8 checks passed
@mshanemc mshanemc deleted the wr/esm branch November 2, 2023 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request USER STORY
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants