Skip to content

[SEMVER-MAJOR] add no-extra-semi - #30

Merged
jannyHou merged 1 commit into
masterfrom
semicolon
Aug 28, 2018
Merged

[SEMVER-MAJOR] add no-extra-semi#30
jannyHou merged 1 commit into
masterfrom
semicolon

Conversation

@jannyHou

Copy link
Copy Markdown
Contributor

Description

A follow up PR based on comment strongloop/generator-loopback#363 (comment)

@bajtos bajtos changed the title add no-extra-semi [SEMVER-MAJOR] add no-extra-semi Aug 28, 2018

@bajtos bajtos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please note this is a breaking change that should be released in a semver-major release.

Comment thread eslint.json Outdated
"quotes": ["error", "single", "avoid-escape"],
"semi-spacing": ["error", {"before": false, "after": true}],
"semi": ["error", "always"],
"no-extra-semi": "error",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe we are keeping the options sorted by alphabet, i.e. no-extra-semi should be inside the group of other no-* options.

@jannyHou

Copy link
Copy Markdown
Contributor Author

Please note this is a breaking change that should be released in a semver-major release.

@bajtos I see. Yeah we should upgrade all LB3 modules by applying this new rule after releasing a major version.

@bajtos bajtos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit:

@jannyHou
jannyHou merged commit 2ebca4c into master Aug 28, 2018
@rmg

rmg commented Aug 29, 2018

Copy link
Copy Markdown
Contributor

All of the downstream CI failures here indicate failures that will happen when this PR is merged. To properly reflect the semver-major nature of the change, package.json should have been modified to actually bump the major version.

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.

4 participants