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

Health: Add clippy linting #269

Merged
merged 8 commits into from
Dec 14, 2023
Merged

Health: Add clippy linting #269

merged 8 commits into from
Dec 14, 2023

Conversation

ryutamago
Copy link
Collaborator

@ryutamago ryutamago commented Dec 12, 2023

Context

This PR introduces a new command make lint that uses clippy and apply them during pre-push / pipeline.

Additionally, i refactored the code so that there will be no linting warnings. The first commit is auto generated by cargo clippy and i manually fixed the linting issues in the last commit.

Make sure you run scripts/install-hooks.sh to install the pre-push-hook script.

Asana Task: Add linting to ci/pre-push hook

We also disable clippy::expect-fun-call rule due to the issue here.

Manually testing the PR

  • pipeline: we can see here that the lint job throws error
  • pre-push-hook: run scripts/install-hooks.sh to install the pre-push-hook script and try pushing with warnings.

@ryutamago ryutamago changed the title Add lint check Infra: Add linting on pipeline/pre-push hook Dec 12, 2023
@ryutamago ryutamago force-pushed the add-lint-check branch 2 times, most recently from 53bd935 to 8d38181 Compare December 13, 2023 10:04
@ryutamago ryutamago changed the title Infra: Add linting on pipeline/pre-push hook Infra: Add make lint command / refactor Dec 13, 2023
@ryutamago ryutamago marked this pull request as ready for review December 13, 2023 10:26
@ryutamago ryutamago requested review from johnyob, alanmarkoTrilitech and sam-finch-tezos and removed request for johnyob December 13, 2023 10:27
crates/jstz_api/tests/wpt.rs Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@johnyob johnyob changed the title Infra: Add make lint command / refactor Health: Add clippy linting Dec 13, 2023
@johnyob
Copy link
Collaborator

johnyob commented Dec 13, 2023

It would also be good to split the existing make check target into calling fmt and lint

@ryutamago ryutamago requested a review from johnyob December 13, 2023 13:42
Copy link
Collaborator

@johnyob johnyob left a comment

Choose a reason for hiding this comment

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

LGTM

crates/jstz_core/src/realm.rs Outdated Show resolved Hide resolved
@ryutamago ryutamago merged commit 79c29ba into main Dec 14, 2023
7 checks passed
@ryutamago ryutamago deleted the add-lint-check branch December 14, 2023 11:26
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