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

style: fix a few pylint alerts #421

Merged
merged 1 commit into from
Jul 26, 2023
Merged

style: fix a few pylint alerts #421

merged 1 commit into from
Jul 26, 2023

Conversation

nedbat
Copy link
Contributor

@nedbat nedbat commented Jul 26, 2023

I think pylint hasn't been run here in a very long time. This removes pep8 which isn't called correctly, and fixes a few actual issues in modules I've touched recently.

@nedbat nedbat requested a review from feanil July 26, 2023 16:12
Makefile Outdated
@@ -38,6 +38,5 @@ upgrade: $(COMMON_CONSTRAINTS_TXT) ## update the requirements/*.txt files with
pip-compile --upgrade -o requirements/development.txt requirements/development.in
for fextra in edx_repo_tools/*/extra.in; do pip-compile --upgrade -o $${fextra%.in}.txt $$fextra; done

lint: ## run pep8 and pylint
pep8 || true
lint: ## run pylint
pylint *.py edx_repo_tools tests || true
Copy link
Contributor

Choose a reason for hiding this comment

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

What's with the "Or True" here, should that be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, it was being used to let targets succeed and ignore the exit status. It was important on pep8 so that pylint would also run, but we don't need it here.

I think pylint hasn't been run here in a very long time.  This removes
pep8 which isn't called correctly, and fixes a few actual issues in
modules I've touched recently.
@nedbat nedbat merged commit 170c895 into master Jul 26, 2023
2 checks passed
@nedbat nedbat deleted the nedbat/clean-up branch July 26, 2023 16:34
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.

2 participants