Aryan29/week1 #4
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Aryan29/week1
aryan29
force-pushed
the
aryan29/week1
branch
from
eeeebb5
to
519edee
Compare
karngyan
reviewed
Dec 21, 2019
|
There are too many bad whitespace pylint errors. I'd recommend using a linter. |
|
Also, do add a .gitignore for pycache |
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
Week-1/aryan29/api.py
Outdated
| def generate_json(self,mob): | ||
| list=[] | ||
| try: | ||
| f=open("data.json","r") |
There was a problem hiding this comment.
Single character variable is highly discouraged.
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
karngyan
reviewed
Dec 21, 2019
Week-1/aryan29/api.py
Outdated
| cprint(f"{mob1} is better than {mob2} comparing all the features",'cyan') | ||
|
|
||
| if __name__ == "__main__": | ||
| if(len(sys.argv)>1): |
There was a problem hiding this comment.
Are you familiar with 'argparse'?
karngyan
reviewed
Dec 21, 2019
Week-1/aryan29/keeptrack.py
Outdated
| if(c%8==1): | ||
| checkserver() #1day updates | ||
| time.sleep(updation_time) | ||
| time.sleep(updation_time) |
There was a problem hiding this comment.
I'm not reviewing each line. But these are the general errors I saw after having a look:
- Some lines seem too long to follow PEP8
- Documentation is None.
- Please defer from using multiple imports in a single line.
- For constants, using UPPER_CASE should be the way to go.
- Pick a style.
- Some imports are unused.
- Correct your import order.
- Few variables are unused. e.g.:
res fis used in two scopes.
There was a problem hiding this comment.
Now using PEP8
Import Rules Done
Line Too Long Done
Using mixedCase for Variables, snake_case for functions and UPPER_CASE for constants
Unused variables and functions removed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.