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

AM - 542: Add custom pre-commit hook #138

Merged
merged 6 commits into from Oct 11, 2023
Merged

AM - 542: Add custom pre-commit hook #138

merged 6 commits into from Oct 11, 2023

Conversation

0xquark
Copy link
Member

@0xquark 0xquark commented Oct 3, 2023

The problem is that when we run the pre-commit script and there's some reformatting needed, we have to run it and stage the changes repeatedly. I've found a simpler solution: I created a special pre-commit hook in the .git/hooks/ folder. This hook checks for changes and keeps running the pre-commit script until all the necessary reformatting is finished.

To use this updated pre-commit hook script, you can create a symbolic link to it in your local .git/hooks directory using this command: ln -s ../../.git-hooks/pre-commit .git/hooks/pre-commit.

I'm also sharing an example of how to use this hook in action:

image

P.S. I added autoflake to the pre-commit-config.yaml file to handle imported but unused errors that come up when running the class_generator. Autoflake automatically removes these unnecessary imports (Also might be the reason that it removed the unnecessary pass statement)

Let me know what you think of this solution.

@0xquark 0xquark requested a review from cmgrote October 3, 2023 14:31
Signed-off-by: Karanjot Singh <drquark@duck.com>
.git-hooks/pre-commit Outdated Show resolved Hide resolved
@cmgrote
Copy link
Collaborator

cmgrote commented Oct 4, 2023

@ErnestoLoma thoughts? 🙏

.git-hooks/pre-commit Outdated Show resolved Hide resolved
Signed-off-by: Karanjot Singh <drquark@duck.com>
@ErnestoLoma ErnestoLoma self-requested a review October 5, 2023 05:48
Copy link
Collaborator

@ErnestoLoma ErnestoLoma left a comment

Choose a reason for hiding this comment

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

I think we can give a try

@cmgrote cmgrote merged commit 7fc8495 into main Oct 11, 2023
3 checks passed
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.

None yet

3 participants