search

openedx / i18n-tools

Tools to help with internationalization and localization of Open edX projects
Apache License 2.0
26 stars 31 forks source link

Update pylintrc to using edx_lint generated version #117

Closed sarina closed 2 years ago

sarina commented 2 years ago

Currently I am unable to update pylintrc to the generated version defined in edx_lint because it was previously manually edited. by @Jawayria @robrap & @UsamaSadiq

Work should be done to upgrade pylintrc properly and resolve the errors that happen because of the upgrade.

https://github.com/openedx/edx-lint

robrap commented 2 years ago

@UsamaSadiq: Can you or your team take care of the upgrade?

UsamaSadiq commented 2 years ago

Hi @robrap @sarina Sure, Arbi-bom will have a look at it tomorrow. We usually take care of not updating the pylintrc manually but seems like it was missed in review along with other major refactoring.

sarina commented 2 years ago

Thanks! There's a few other similar ones I've filed, and I am pretty sure I will encounter more as I've only processed about 50 of my 184 open PRs.

https://github.com/openedx/super-csv/issues/118 https://github.com/openedx/xblock-adventure/issues/49 https://github.com/openedx/taxonomy-connector/issues/110 https://github.com/openedx/staff_graded-xblock/issues/125

sarina commented 2 years ago

I solved this for taxonomy-connector by matching pylintrc_tweaks to edx-platform (with the addition of no-self-use, which is new, and I'm working on adding to edx-platform's pylintrc_tweaks file).

See https://github.com/openedx/taxonomy-connector/pull/108/commits/8a393d771dee780a50533c1f89c01c4c6f4abc88

robrap commented 2 years ago

@nedbat: Additional thoughts about tweaks:

  1. Is there a how-to for promoting a tweak to be a part of the default settings?
  2. Should we comment tweaks to help understand context?
  3. Do tweaks get cleaned up if duplicating default settings? Manual or automatic?

Context: Sarina was talking about copying around tweaks, and that made me think that maybe these tweaks shouldn't be tweaks, but I did not look into the details.

nedbat commented 2 years ago

I think we are suffering a lot of R0201 (no-self-use) these days. It might make sense to move that disable into the central file. Pylint seems fine with mentioning the same message id in the disable list twice, so it shouldn't hurt the early tweakers to disable the message centrally.

nedbat commented 2 years ago

For the other questions: there's no how-to for moving a setting to the central file, and there's no agreed process for deciding what goes into the central file. Commenting tweaks sounds like a great idea to me.

sarina commented 2 years ago

Hmm! https://pylint.pycqa.org/en/latest/whatsnew/2/2.14/summary.html#removed-checkers

Moved no-self-use check to optional extension. You now need to explicitly enable this check using load-plugins=pylint.extensions.no_self_use.

UsamaSadiq commented 2 years ago

As per the discussions, I'll create a separate PR in edx-lint to move the no-self-use check to central pylintrc disabled section.