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

chore: add isort as pre-commit hook #319

Merged
merged 8 commits into from
Jul 6, 2023

Conversation

Mustaballer
Copy link
Collaborator

@Mustaballer Mustaballer commented Jun 26, 2023

What kind of change does this PR introduce?

Adds pre-commit hooks namely isort, check-yaml, end-of-file-fixer, trailing-whitespace.

Summary

At the moment, OpenAdapt does not provide an automated way to sort imports. However, we have incorporated the isort pre-commit hook, which detects unsorted imports and rejects a commit if they are found. If any unsorted imports are detected, isort will automatically modify the affected files to sort the imports correctly. This helps maintain consistent import organization across the codebase. The other hooks I added I believe are useful and are listed below.

check-yaml: Validates the syntax and structure of YAML files.
end-of-file-fixer: Ensures that files end with a newline character.
trailing-whitespace: Detects and removes trailing whitespace at the end of lines.
isort: Sorts Python import statements in a consistent and standardized manner.

Checklist

  • My code follows the style guidelines of OpenAdapt
  • I have perfomed a self-review of my code
  • If applicable, I have added tests to prove my fix is functional/effective
  • I have linted my code locally prior to submission
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (e.g. README.md, requirements.txt)
  • New and existing unit tests pass locally with my changes

How can your code be run and tested?

  1. poetry install
  2. pre-commit install note: in root directory
  3. Make a commit making changes to some files(only staged files with changes will be executed with the hooks)

@Mustaballer Mustaballer requested a review from abrichr June 26, 2023 16:29
@Mustaballer Mustaballer changed the title Add isort as pre-commit hook chore: add isort as pre-commit hook Jun 30, 2023
@Mustaballer
Copy link
Collaborator Author

Ready for Review :)

Copy link
Member

@abrichr abrichr left a comment

Choose a reason for hiding this comment

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

Thanks @Mustaballer ! Just a few small things 🙏

README.md Outdated
@@ -226,6 +226,29 @@ In summary (from https://stackoverflow.com/a/69673312):
alembic revision --autogenerate -m "<msg>"
```

### Pre-commit Hooks

To ensure code quality and consistency, OpenAdapt uses pre-commit hooks. These hooks will be executed automatically before each commit to perform various checks and validations on your codebase.
Copy link
Member

Choose a reason for hiding this comment

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

Can we also add a pre-commit hook to split long lines like this one onto multiple lines? 😅

Copy link
Collaborator Author

@Mustaballer Mustaballer Jul 4, 2023

Choose a reason for hiding this comment

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

Unfortunately there is no pre-commit hook that does this from what I researched. @abrichr I was also considering adding ruff(popular and fast python linter) as a pre-commit hook.

Copy link
Member

Choose a reason for hiding this comment

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

README.md Outdated Show resolved Hide resolved
@abrichr abrichr merged commit 72da341 into OpenAdaptAI:main Jul 6, 2023
R-ohit-B-isht pushed a commit to R-ohit-B-isht/OpenAdapt that referenced this pull request Jun 21, 2024
* Add precommit hooks for isort, check-yaml, end-of-file-fixer, trailing-whitespaces

* Updated README on setting up precommit hooks

* Add updated poetry.lock

* Fix step numbering of pre-commit section in README.md
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