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

feat: Add pre-commit configuration for code quality checks #258

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

areebahmeddd
Copy link
Member

Signed-off-by: Areeb Ahmed [email protected]

Summary of Changes

  • Added .pre-commit-config.yaml file to configure pre-commit hooks.
  • Configured hooks for:
    • Fixing end-of-file issues.
    • Removing trailing whitespace.
    • Checking JSON and YAML file validity.
    • Running Ruff for linting and formatting.

Related Issues

#256

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.69%. Comparing base (b9cda65) to head (12b498a).
Report is 75 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #258      +/-   ##
==========================================
- Coverage   95.06%   94.69%   -0.37%     
==========================================
  Files           5        5              
  Lines         324      377      +53     
==========================================
+ Hits          308      357      +49     
- Misses         16       20       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@alexgarel
Copy link
Member

@areebahmeddd I see it's draft, but just in case:

  • document the tool
  • make it so that it must not be mandatory to use pre-commit hook (we don't want to put too high a barrier to contributing), so also explain how people can link their code without it
  • ensure that CI runs linting rules (otherwise we will have PR's with conflicting edit from people using pre-commit hooks)

@areebahmeddd
Copy link
Member Author

Yes, need to document it for contributors. Also, should I run the pre-commit and add the clean files, or just add it to github ci so it runs on future merges?

CC: @alexgarel

@alexgarel
Copy link
Member

@areebahmeddd I think you we must have CI within this PR and reformatted code… (otherwise, once again it will only lead to conflict between people using pre-commit hook).

But it's on your side, to resolve conflicts (the best would be to keep reformating commits separated, so you can remove them, and replay formating on the go).
It's up to you also to handle communication as soon as this PR is merged (to ask other to merge main and run linting on their code).

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