Skip to content

Reformat codebase and add pre-commit #81

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

Merged
merged 12 commits into from
Jun 25, 2021
Merged

Reformat codebase and add pre-commit #81

merged 12 commits into from
Jun 25, 2021

Conversation

zhijian-liu
Copy link
Contributor

@zhijian-liu zhijian-liu commented Jun 19, 2021

This PR reformats the codebase and integrates pre-commit for continuous integration. Contributors can first activate it locally by pre-commit install, which will then be triggered automatically at git commit to check, fix and reformat the commit.

@zhijian-liu zhijian-liu force-pushed the dev/pre-commit branch 3 times, most recently from 81179bc to a57dc9f Compare June 21, 2021 10:38
kentang-mit
kentang-mit previously approved these changes Jun 21, 2021
clee-ai
clee-ai previously approved these changes Jun 21, 2021
Copy link
Collaborator

@clee-ai clee-ai left a comment

Choose a reason for hiding this comment

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

Looks great! I'm curious, what's the advantage of doing it through pre-commit versus the old formatter workflow?

@zhijian-liu
Copy link
Contributor Author

zhijian-liu commented Jun 21, 2021

@CCInc, the advantages of pre-commit are mainly two-fold:

sandeepnmenon
sandeepnmenon previously approved these changes Jun 21, 2021
Copy link
Collaborator

@sandeepnmenon sandeepnmenon left a comment

Choose a reason for hiding this comment

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

This is great!

@zhijian-liu
Copy link
Contributor Author

@CCInc @sandeepnmenon, this PR is ready for review. @kentangSJTU, I've tested the correctness of this version on semantic segmentation. It would be great if you could test it on object detection as well.

@zhijian-liu zhijian-liu force-pushed the dev/pre-commit branch 2 times, most recently from 4a235ba to 7aa420d Compare June 23, 2021 07:22
Copy link
Collaborator

@clee-ai clee-ai left a comment

Choose a reason for hiding this comment

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

Great! I love how clean the pytorch API is now.

@zhijian-liu zhijian-liu changed the title Use pre-commit for continuous integration Reformat codebase and add pre-commit Jun 24, 2021
@zhijian-liu zhijian-liu merged commit 74099d1 into master Jun 25, 2021
@zhijian-liu zhijian-liu deleted the dev/pre-commit branch June 25, 2021 01:02
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.

4 participants