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

Move linting checks to start of CI run #1059

Merged
merged 6 commits into from
Dec 1, 2021
Merged

Move linting checks to start of CI run #1059

merged 6 commits into from
Dec 1, 2021

Conversation

HCastano
Copy link
Contributor

@HCastano HCastano commented Nov 30, 2021

It's really annoying to wait half an hour while the CI does its job, only to realize that
you forgot to run rustfmt and now you have to wait another half an hour for the CI to
pass.

This PR moves our linting steps to the beginning of the CI pipeline so that these
failures can be caught and addressed quickly.

This PR also allows the CI build to continue despite the lint steps failing. This is
useful if you care about a step later in the CI pipeline (e.g contract sizes) but don't
necessarily care if your code is formatted properly.

@HCastano HCastano marked this pull request as ready for review November 30, 2021 19:25
@HCastano HCastano requested review from ascjones, cmichi, Robbepop and a team as code owners November 30, 2021 19:25
@codecov-commenter
Copy link

Codecov Report

Merging #1059 (2909953) into master (78b4533) will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1059      +/-   ##
==========================================
+ Coverage   78.75%   78.79%   +0.04%     
==========================================
  Files         248      248              
  Lines        9371     9371              
==========================================
+ Hits         7380     7384       +4     
+ Misses       1991     1987       -4     
Impacted Files Coverage Δ
...ates/storage/src/collections/hashmap/fuzz_tests.rs 100.00% <0.00%> (+4.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 78b4533...2909953. Read the comment docs.

#### stage: lint
#
# Note: For all of these lints we `allow_failure` so that the rest of the build can
# continue running despite them not passing. Merging is still disallowed since (most) of
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just as an FYI: I checked and it's only spellcheck which is currently not required for merging.

Copy link
Collaborator

@cmichi cmichi left a comment

Choose a reason for hiding this comment

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

Let's try it out.

@cmichi cmichi merged commit 44c51d1 into master Dec 1, 2021
@cmichi cmichi deleted the hc-improve-ci-order branch December 1, 2021 06:48
xgreenx pushed a commit to Supercolony-net/ink that referenced this pull request Feb 8, 2022
* Move linting checks to start of CI run

* Test RustFmt failure

* Test Clippy quickfail

* Try adding `allow_failure` to Clippy jobs

* Allow lint steps to fail in the GitLab CI

* Revert test failures in `flipper`
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