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

pylintrc: adjust configuration to labgrid needs #257

Merged
merged 1 commit into from
Jun 4, 2018

Conversation

Bastian-Krause
Copy link
Member

no-member

Pretty useless, because attr is used pretty much everywhere and pylint does not recognize it. Ignore this.

arguments-differ

Not sure if we should check this, e.g. almost every driver implements a run method with different arguments.

useless-super-delegation

The docs state that this is needed:
"The minimum requirement is a call to super().__attrs_post_init__()." Why do we override it to call the method on super?

no-self-use

I think wrapping functionality in methods that could be functions is still a good thing for the sake of logic. Ignore this.

protected-access

Not sure if we should check this, there are quite some occurrences across labgrid. Maybe this is by design or we need to define exceptions in file headers for this.

duplicate-code

Detects only by-design boilerplate code, ignore it.

max-attributes=10

Increase this, 10 should be okay too.

min-public-methods=0

Decrease this to fit labgrid's inheritance model.

max-parents=10

Increase this, having a lot of parents is needed by design.

@codecov-io
Copy link

codecov-io commented Jun 4, 2018

Codecov Report

Merging #257 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #257   +/-   ##
======================================
  Coverage    54.9%   54.9%           
======================================
  Files         105     105           
  Lines        6229    6229           
======================================
  Hits         3424    3424           
  Misses       2805    2805

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3dd87f5...0ba39ab. Read the comment docs.

@Emantor
Copy link
Member

Emantor commented Jun 4, 2018

We definitely want arguments-differ. This has been a source of problem in recent changes, it would greatly help to keep arguments the same across drivers.

@Bastian-Krause
Copy link
Member Author

Okay, nice. I have not added it the commit because I was unsure - so no need to change. What about the other changes?

@Emantor
Copy link
Member

Emantor commented Jun 4, 2018

no-member

We don't get around disabling this for attr.s.

useless-super-delegation:

python-attrs/attrs#167

attrs does not provide inheritance class initiliazation, so we have to do it ourselves.

no-self-use

we should at least make those @staticmethod

protected-access

We should fix those occurences and/or provide a public interface.

duplicate-code

If it is only boilerplate code it sounds good, otherwise it needs to be fixed too.

IMO increasing the attribute numbers seems reasonable.

@jluebbe what is your opinion here?

@jluebbe
Copy link
Member

jluebbe commented Jun 4, 2018

I agree with @Emantor here.

@Bastian-Krause
Copy link
Member Author

Oh, I did not know that attrs requires explicit super delegation and had no idea Python has a @staticmethod decorator - nice.

Check the linting results of PR #255 here. If this is okay with you, I will modify this PR accordingly.

@Bastian-Krause Bastian-Krause changed the title RFC: pylintrc: adjust configuration to labgrid needs pylintrc: adjust configuration to labgrid needs Jun 4, 2018
Signed-off-by: Bastian Stender <bst@pengutronix.de>
@Bastian-Krause
Copy link
Member Author

  • added: no-member
  • removed: no-self-use, protected-access (see comments above)

@Emantor Emantor self-requested a review June 4, 2018 15:34
@jluebbe jluebbe merged commit 8289085 into labgrid-project:master Jun 4, 2018
@Bastian-Krause Bastian-Krause deleted the bst/pylintrc branch September 14, 2022 15:39
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.

4 participants