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

Update and improve .pre-commit-config.yaml #748

Merged
merged 6 commits into from
Jun 13, 2024
Merged

Conversation

phklive
Copy link
Contributor

@phklive phklive commented Jun 12, 2024

In this PR I propose to update and improve the .pre-commit-config.yaml file that we use in our repo.

I have also added documentation and made sure that it uses our Makefile commands.

Closes: #537

based on #746

@phklive phklive marked this pull request as ready for review June 12, 2024 13:25
@phklive phklive requested review from bobbinth and igamigo and removed request for bobbinth June 12, 2024 13:25
README.md Outdated
Comment on lines 73 to 75
## Commiting

To make sure all commits adhere to our programming standards we use [pre-commit](https://pre-commit.com/) ([file](.pre-commit-config.yaml)) to run automatic commands on each commit. Please install it and follow the setup instructions for your machine.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this go into the CONTRIBUTING file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed we can move it there.

Makefile Outdated
Comment on lines 67 to 68
check: ## Check all targets and features
cargo check --all-targets $(ALL_FEATURES_BUT_ASYNC)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would make the comment a bit more informative. Maybe something like "Check all targets and features for errors without code generation"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

@phklive phklive requested a review from bobbinth June 13, 2024 11:14
Base automatically changed from phklive-base-doc to next June 13, 2024 17:43
@bobbinth bobbinth force-pushed the phklive-pre-commit-config branch from f6882d1 to 484ab51 Compare June 13, 2024 18:12
@phklive
Copy link
Contributor Author

phklive commented Jun 13, 2024

@bobbinth just to understand, why did you force push?

@bobbinth
Copy link
Contributor

@bobbinth just to understand, why did you force push?

Since I merged #746 first, this needed to be rebased.

Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you!

@bobbinth bobbinth merged commit aa4bd1a into next Jun 13, 2024
12 checks passed
@bobbinth bobbinth deleted the phklive-pre-commit-config branch June 13, 2024 18:34
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