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

Add tests to follow PEP8 guidelines #226

Open
devkapilbansal opened this issue Mar 25, 2021 · 15 comments
Open

Add tests to follow PEP8 guidelines #226

devkapilbansal opened this issue Mar 25, 2021 · 15 comments
Labels
Category: Outreach/Research Changes or improvements related to external research of the app. Status: Available Issue was approved and available to claim or abandoned for over 3 days.

Comments

@devkapilbansal
Copy link
Member

Is your feature request related to a problem? Please describe.

PEP stands for Python Enhancement Proposal. It is a style document that documents some best practices

Describe the solution you'd like

Code written in Python should follow standard practices and PEP8 guidelines. For this a linter that follows these guidelines or some sort of checks can be implemented

Describe alternatives you've considered

I would suggest using Flake8 to test if PEP8 best practices are followed in the code written or not. Additionally, Flake8 workflow tests can be setup to ensure pull requests made follow these guidelines as well.

Additional context

Flake8 is not the only tool available and other alternatives can be considered.

@devkapilbansal devkapilbansal added Category: Outreach/Research Changes or improvements related to external research of the app. Open Source Hack Status: Available Issue was approved and available to claim or abandoned for over 3 days. labels Mar 25, 2021
@Aryamanz29
Copy link
Contributor

I would like to work on this one @devkapilbansal

@decon-harsh
Copy link
Member

I would like to work on this one @devkapilbansal

You have been already assigned one issue. Please claim it after you send a successful PR for the former.

@epicadk epicadk removed the Status: Available Issue was approved and available to claim or abandoned for over 3 days. label Apr 1, 2021
@devkapilbansal
Copy link
Member Author

@Aryamanz29 are you working on this?

@Aryamanz29
Copy link
Contributor

@Aryamanz29 are you working on this?

Yes,
Sorry for the delay I'll raise PR soon.

Aryamanz29 added a commit to Aryamanz29/bridge-in-tech-backend that referenced this issue Apr 8, 2021
Aryamanz29 added a commit to Aryamanz29/bridge-in-tech-backend that referenced this issue Apr 8, 2021
Aryamanz29 added a commit to Aryamanz29/bridge-in-tech-backend that referenced this issue Apr 8, 2021
Aryamanz29 added a commit to Aryamanz29/bridge-in-tech-backend that referenced this issue Apr 8, 2021
Aryamanz29 added a commit to Aryamanz29/bridge-in-tech-backend that referenced this issue Apr 8, 2021
Aryamanz29 added a commit to Aryamanz29/bridge-in-tech-backend that referenced this issue Apr 8, 2021
Aryamanz29 added a commit to Aryamanz29/bridge-in-tech-backend that referenced this issue Apr 8, 2021
Aryamanz29 added a commit to Aryamanz29/bridge-in-tech-backend that referenced this issue Apr 8, 2021
@Aryamanz29
Copy link
Contributor

@devkapilbansal While testing my code locally most unittest fails and I'm unable to debug it after several attempts, So kindly unassign this issue to me for now. I'll try to work on this issue later.

Screenshot (49)

@devkapilbansal devkapilbansal added the Status: Available Issue was approved and available to claim or abandoned for over 3 days. label Apr 9, 2021
@epicadk
Copy link
Member

epicadk commented Apr 9, 2021

Are you using @mtreacy002's forked repo as the Mentorship backend?

@mtreacy002
Copy link
Member

mtreacy002 commented Apr 10, 2021

@Aryamanz29 , can you check if running the test from the Test Explorer of your VS Code IDE pass? I've asked since this is also happened to me when I'm using Windows OS (running tests inside the terminal failed but running them inside VS Code IDE Test explorer passed). I have opened an issue #133 to investigate this. ATM when using Windows OS, I just run the tests on the VS Code terminal. On MacOS this issue doesn't exist (both terminal and VS Code IDE work just fine). cc @epicadk

@Aryamanz29
Copy link
Contributor

@mtreacy002 Test fails same as they on the terminal, Now I'll try to set up this repo on my Linux machine to solve this problem😅

Screenshot (51)

@mtreacy002
Copy link
Member

mtreacy002 commented Apr 10, 2021

@Aryamanz29 , also, have you completed the compulsory #244 before working on this issue? You shouldn’t face this issue once you’ve successfully completed the task there. cc @epicadk

@epicadk
Copy link
Member

epicadk commented Apr 10, 2021

@Aryamanz29 , also, have you completed the compulsory #244 before working on this issue? You shouldn’t face this issue once you’ve successfully completed the task there. cc @epicadk

I guess this was before I was your message 😅. @Aryamanz29 please complete #244 before you attempt this.

@Aryamanz29
Copy link
Contributor

@Aryamanz29 , also, have you completed the compulsory #244 before working on this issue? You shouldn’t face this issue once you’ve successfully completed the task there. cc @epicadk

I guess this was before I was your message 😅. @Aryamanz29 please complete #244 before you attempt this.

I have raised PR #267 for env setup, Please have a look to it @epicadk

@mtreacy002
Copy link
Member

mtreacy002 commented May 22, 2021

@devkapilbansal and @epicadk , since we're running lint workflow using black, wouldn't this pr (using Flake8) contradicting the workflow. I mean the 2 (black and flake8) don't always play nice with each other 🤔 . Unless you're thinking of using flake8-black plugin, perhaps on the VSCode IDE? But I still don't see why we need to add flake8 workflow on top of black linting 🤔

@devkapilbansal
Copy link
Member Author

Hi @mtreacy002 black is used to lint the code so to increase readability but flake8 ensures that the code follows Python standards specified by PEP8.
There are some cases where black and flake8 contradicts each other but I think that can be managed using config file.

@mtreacy002
Copy link
Member

Hi @mtreacy002 black is used to lint the code so to increase readability but flake8 ensures that the code follows Python standards specified by PEP8.
There are some cases where black and flake8 contradicts each other but I think that can be managed using config file.

May I ask what you think of the flake8-black plugin I mentioned above? wouldn't this be a faster and easier option instead of writing the config file, @devkapilbansal ?

@devkapilbansal
Copy link
Member Author

Yes, it looks good to. I was unaware of this. I think we can proceed with this instead of having flake8 and flask.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Outreach/Research Changes or improvements related to external research of the app. Status: Available Issue was approved and available to claim or abandoned for over 3 days.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants