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

ENHANCE: Use pylint for Surround's linter #206

Merged
merged 13 commits into from
Nov 19, 2019

Conversation

zacbrannelly
Copy link
Collaborator

@zacbrannelly zacbrannelly commented Oct 24, 2019

  • Rip out previous linter and replace with Pylint
  • Setup default disabled messages and fix generated projects

@zacbrannelly zacbrannelly changed the title [WIP] ENHANCE: Use pylint for Surround's linter ENHANCE: Use pylint for Surround's linter Oct 25, 2019
Copy link

@mpej mpej left a comment

Choose a reason for hiding this comment

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

When running linter on autogenerated web-runner project, the initial rating is 9.77/10
image

EDIT: this was not specific to this PR. I made #219 to fix this.

Copy link
Member

@ucokzeko ucokzeko left a comment

Choose a reason for hiding this comment

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

@zacbrannelly Can you please rebase this again? Sorry

I also got this issue from generating this project from this PR

✔ ~/A2I2/Code/temp/linter_test
09:24 $ surround lint
************* Module dodo
dodo.py:129:0: C0301: Line too long (106/100) (line-too-long)
dodo.py:165:0: C0301: Line too long (106/100) (line-too-long)

------------------------------------------------------------------
Your code has been rated at 9.91/10 (previous run: 9.91/10, +0.00)

@zacbrannelly
Copy link
Collaborator Author

@ucokzeko Fixed and added a test case to check for linter errors in the generated project

Copy link
Member

@ucokzeko ucokzeko left a comment

Choose a reason for hiding this comment

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

@zacbrannelly Is it possible to run the same Pylint version that we use from requirement.txt and the version that we install in CircleCi when we run the test? Try to find a way to install it from 1 source instead of updating the version in the config file.

@zacbrannelly
Copy link
Collaborator Author

@ucokzeko Yeah I just removed installing pylint from the CircleCI config, this way it will just use the version that is in the requirements.txt

Copy link
Member

@ucokzeko ucokzeko left a comment

Choose a reason for hiding this comment

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

LGTM

@ucokzeko ucokzeko merged commit 1d7e508 into a2i2:master Nov 19, 2019
@zacbrannelly zacbrannelly deleted the enhance/pylint branch November 19, 2019 22:33
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.

3 participants