-
Notifications
You must be signed in to change notification settings - Fork 20
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
Flake8 Linter added #49
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hellow @soham4abc ,
- The linter should only work over the nirjas folder, You can leave the tests/ folder for now.
- Please fix the error in rest of the files.
- Instead of calling it CI workflow, Can you name it as CI-linter etc?
Signed-off-by: Soham Banerjee <sohambanerjee4abc@hotmail.com>
I have done the requested changes except fixing the currently present lint errors. I believe this should be done in a separate issue itself. Well Will be waiting for your review! |
Hey @soham4abc , Yes that should be the right way to do it but How will I merge this PR with the pipeline getting failed? 😞 |
Okay then. But it will take some amount of time please be patient! |
We have all the time in the universe mate!! 🕐 |
Signed-off-by: Soham Banerjee <sohambanerjee4abc@hotmail.com>
All lint errors fixed please do have a look! |
Hey @soham4abc , Thanks for your efforts! I see that you have ignored (W503,E203) in the linters. These are checks that we require. |
Sure! |
I pushed the requested changes Please do have a look @GMishx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @soham4abc , Changes looks good now. Ready to merge.
Thankyou for your contributions.
Happy to help! |
Fixes #39
Flake8 Linter added. Please Do have a look