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

ci: move lint out of build #1255

Merged
merged 1 commit into from
Nov 17, 2023
Merged

ci: move lint out of build #1255

merged 1 commit into from
Nov 17, 2023

Conversation

benjlevesque
Copy link
Contributor

@benjlevesque benjlevesque commented Nov 17, 2023

the format & lint takes about a minute. It shouldn't be part of the build stage, but in a separate lint stage (together with package.json lint)

Comment on lines +99 to +100
paths:
- ./
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just remove a warning from the json schema

lint-package-json:
paths:
- ./
lint:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

rename job to match what it does

Comment on lines +108 to +113
- run:
name: Verify format
command: yarn format:check
- run:
name: Verify lint
command: yarn lint:check
Copy link
Contributor Author

Choose a reason for hiding this comment

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

main change

@@ -659,44 +660,48 @@ workflows:
# Publish the code coverage
- publish-coverage:
requires:
- test-request-client
- lint
Copy link
Contributor Author

Choose a reason for hiding this comment

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

add lint, integration-test and smart-contracts which were missing
sort all tests

Comment on lines -717 to -719
- lint-package-json:
requires:
- build
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unnecessary in nighly build

Copy link
Member

@MantisClone MantisClone left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the excellent comments 👍

@coveralls
Copy link

Coverage Status

coverage: 86.918%. remained the same
when pulling c5c9dd8 on ci/move-lint-out-of-build
into 71b2c46 on master.

Copy link
Member

@alexandre-abrioux alexandre-abrioux left a comment

Choose a reason for hiding this comment

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

👍

@benjlevesque benjlevesque merged commit 07e5cad into master Nov 17, 2023
27 checks passed
@benjlevesque benjlevesque deleted the ci/move-lint-out-of-build branch November 17, 2023 15:40
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.

5 participants