anitab-org / vms

THIS PROJECT IS ARCHIVED. Volunteer Management System.
GNU General Public License v2.0
1 stars 4 forks source link

Fixes #1035: Add black and flake8 as pre-commit hook #1058

Open BALaka-18 opened 4 years ago

BALaka-18 commented 4 years ago

Description

This PR adds the code formatting tools, black and flake8, as pre-commit hook to ensure the code is organized and well formatted.

Fixes #1035

Type of Change:

Code/Quality Assurance Only

How Has This Been Tested?

Screenshots:

Screenshot (339) Screenshot (340)

requirements.txt :

Screenshot (342)

Checklist:

Code/Quality Assurance Only

Kajol-Kumari commented 4 years ago

@BALaka-18 Codewise LGTM! Can you please add the screenshot by runing this pre-commit hook on your local?

BALaka-18 commented 4 years ago

@BALaka-18 Codewise LGTM! Can you please add the screenshot by runing this pre-commit hook on your local?

Yes sure.

BALaka-18 commented 4 years ago

@Kajol-Kumari I have attached the screenshots.

Kajol-Kumari commented 4 years ago

Hey @BALaka-18 can we please add a check to insure that users install the pre-commit hook. Otherwise don't allow them to commit (reject the commit).

BALaka-18 commented 4 years ago

Looks good from a first look, can you post the diff of requirements.txt, I can't seem to see it in the view for some reason

@SanketDG I had removed the older versions of six and PyYAML, else the build was running into errors. Otherwise all same.

BALaka-18 commented 4 years ago

@Kajol-Kumari I have added the check.

Kajol-Kumari commented 4 years ago

Hey @BALaka-18 If I am not wrong, you have just pasted the ss of the changes You have done in requirement.txt file and not the whole file content? (just want to confirm)

BALaka-18 commented 4 years ago

Hey @BALaka-18 If I am not wrong, you have just pasted the ss of the changes You have done in requirement.txt file and not the whole file content? (just want to confirm)

@Kajol-Kumari these are the extra dependencies required to make the pre-commit hook run. The rest of the dependencies are the same as in the current requirements.txt file. So I passed only the new dependencies needed and not the whole file.

Should I upload the ss for the whole file ?

Kajol-Kumari commented 4 years ago

Thanks for confirming @BALaka-18. No, you don't need to attach the ss of the whole file here.

BALaka-18 commented 4 years ago

Hi @Kajol-Kumari, is this PR ready to be merged ? Could you please request a review from a reviewer with write access ? It still says that review is required.

Kajol-Kumari commented 4 years ago

@BALaka-18 please squash all the commits into a single commit so that it can be send for final approval.

BALaka-18 commented 4 years ago

@BALaka-18 please squash all the commits into a single commit so that it can be send for final approval.

@Kajol-Kumari done !