Skip to content

fix syntax issue causing failure on older versions of nodeJS#156

Closed
brettcomardelle wants to merge 1 commit into
loopbackio:masterfrom
brettcomardelle:node-compatibility
Closed

fix syntax issue causing failure on older versions of nodeJS#156
brettcomardelle wants to merge 1 commit into
loopbackio:masterfrom
brettcomardelle:node-compatibility

Conversation

@brettcomardelle

Copy link
Copy Markdown

Description

Running the connector in node v7 and v8 failed due to the trailing commas. Later versions of nodeJS would ignore the syntax issue.

Related issues

  • connect to <link_to_referenced_issue>

Checklist

  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style
    guide

@slnode

slnode commented Oct 24, 2019

Copy link
Copy Markdown

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@dhmlau

dhmlau commented Oct 25, 2019

Copy link
Copy Markdown
Member

@brettcomardelle, thanks for your PR. Your changes LGTM. Could you please fix the commit linter error? Thanks.

**************************************************
**
**  Linting commit logs
**
**  1 problems found:
**    4e8bf1b - fix syntax issue causing failure on older versions : First line should be 50 characters or less (saw 60)
**
**************************************************

@dhmlau

dhmlau commented Oct 25, 2019

Copy link
Copy Markdown
Member

@bajtos bajtos added the community-contribution Patches contributed by community label Nov 1, 2019
@bajtos
bajtos force-pushed the node-compatibility branch from 4e8bf1b to 3bd97b8 Compare November 1, 2019 09:53
@bajtos

bajtos commented Nov 1, 2019

Copy link
Copy Markdown
Member

I have amended the commit message for you, let's wait for CI results before landing.

@bajtos

bajtos commented Nov 1, 2019

Copy link
Copy Markdown
Member

@slnode ok to test

@bajtos

bajtos commented Nov 1, 2019

Copy link
Copy Markdown
Member

The builds are failing on linting, because our eslint config is enforcing trailing commas.

I did a bit of investigation and have few comments:

Node.js 8.0 and newer support trailing commas in function arguments. Here is an example to try:

function x(
  a,
) {
  console.log('ok', a);
}

x(1);

We don't support Node.js 7.x, it has reached end of life way too long time ago.

In that light, I am closing this pull request as rejected.

@bajtos bajtos closed this Nov 1, 2019
@bajtos bajtos mentioned this pull request Nov 1, 2019
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution Patches contributed by community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants