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

fix: upgrade to minimum of node 18 #333

Merged
merged 4 commits into from
Dec 7, 2023
Merged

Conversation

peternhale
Copy link
Collaborator

What does this PR do?

  • Updates node to a minimum of node 18
  • Bump dependencies to more recent versions
  • Migrate husky from v4 to v8
  • fix eslint errors due to changes in new versions of eslint/tslint modules

What issues does this PR fix or reference?

@W-14196513@

Should be no changes in functionality

@@ -13,7 +13,7 @@ import {
soapEnv,
soapBody,
soapHeader,
action
action,
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we change the prettier/eslint behaviour for trailing commas or did they make a default change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

default change

Copy link
Contributor

Choose a reason for hiding this comment

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

@peternhale In our main extensions repo, we changed .prettierrc to have "trailingCommas": false. Since you said this is a default change in Prettier, do we want to do the manual override here as well?

src/streaming/types.ts Outdated Show resolved Hide resolved
@@ -282,6 +282,7 @@ describe('Apex Test Suites', async () => {
'TestClass1.method1,namespace.TestClass2.method1,TestClass2.method2';

const testService = new TestService(mockConnection);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this comment meant to be info or was this a debugging line that should be removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@daphne-sfdc The comment is a directive to eslint to disable the named rule for the next line.

Copy link
Contributor

@daphne-sfdc daphne-sfdc left a comment

Choose a reason for hiding this comment

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

Code changes all look good. Behavior unchanged from before for both Mac and Windows, as expected. QE doc here: https://salesforce.quip.com/1IubA5tO9DmG

…phale/W-14196513-node-18

# Conflicts:
#	src/tests/syncTests.ts
@peternhale peternhale merged commit e87eb9e into main Dec 7, 2023
9 checks passed
@peternhale peternhale deleted the phale/W-14196513-node-18 branch December 7, 2023 17:05
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.

3 participants