Skip to content

Conversation

@individual-it
Copy link
Contributor

run prettier on the code and enable the lint check in CI

@TimothyJones
Copy link
Contributor

Hmm. Lint is already run by the build, and lint runs prettier, so if the examples weren't formatted already the build should fail. Probably we have a misconfiguration where the formatter has different settings in checking vs formatting for some reason.

@TimothyJones
Copy link
Contributor

Looks like linting has been disabled since march in the V3 branch, in 6afc281 . I'm not sure why, but we'll need to re-enable it before merging V3 in.

@individual-it
Copy link
Contributor Author

in this branch it only ran for build:v2 not build:v3 7d76c63
and the build script only runs build:v3 https://github.com/pact-foundation/pact-js/blob/feat/v3.0.0/scripts/ci/build-and-test.sh#L11

"build": "npm run build:v2 && npm run build:v3",
"build:v2": "npm run lint && npm run compile",
"build:v3": "neon build --release",
"build:v3": "npm run lint && neon build --release",
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm. I'm not sure this is the right place for it either, as neon build doesn't use anything that would be linted.

I think on master probably the compile script should move into the dist script (and be removed), and lint is then a predist script.

We're really due an overhaul of all these scripts, there are many more than is necessary.

@TimothyJones
Copy link
Contributor

in this branch it only ran for build:v2 not build:v3

Well spotted. However, since the commit I linked, the lint script doesn't run lint any more, it only runs the formatter.

@TimothyJones TimothyJones mentioned this pull request Jan 22, 2021
@individual-it individual-it deleted the lintingCode branch January 25, 2021 03:19
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.

2 participants