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

Added disable pointless-string-statement check patch to update pylintrc files #202

Closed
wants to merge 2 commits into from

Conversation

evaherrada
Copy link
Collaborator

@evaherrada evaherrada commented Jan 11, 2021

@sommersoft @jepler Since you both have done this sort of thing in the past, do you think you might be able to look over this patch to see if it's correct? Thanks!

@evaherrada evaherrada requested review from sommersoft and jepler and removed request for sommersoft January 11, 2021 20:09
Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

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

Can you provide more info about why we want to disable this check?

Can you provide a link to a repo where this has been manually applied and CI passes?

@evaherrada
Copy link
Collaborator Author

@jepler I disabled it in this pr (adafruit/Adafruit_CircuitPython_BNO055#68). The reason I think we should disable it is that when it is enabled, Pylint doesn't let you use triple quote comments outside of docstrings.

Here's a discussion on the subject: pylint-dev/pylint#2864

@sommersoft
Copy link
Collaborator

I've been away for a while, but I'll add some thoughts on this.

It doesn't surprise me that pylint is rather rigid on non-docstring/unassigned multi-line strings. They're pretty strict in their adherence to PEP8 (GvR's Twitter post, not withstanding); # is the standard for comments.

I would be wary of allowing the different styles to coexist in the entire ecosystem. Even in the single file examples/bno055_simpletest.py referenced in the BNO055 PR, there are two methods used:

# User these lines for UART
# uart = busio.UART(board.TX, board.RX)	# uart = busio.UART(board.TX, board.RX)
# sensor = adafruit_bno055.BNO055_UART(uart)	# sensor = adafruit_bno055.BNO055_UART(uart)

and

"""
print(
    "Temperature: {} degrees C".format(temperature())
)  # Uncomment if using a Raspberry Pi
"""

Other than that, the patch file looks fine. A dry run would point out any issues/skipped repos.

@tannewt
Copy link
Member

tannewt commented Jan 12, 2021

I agree with @sommersoft. We shouldn't change the lint settings. The BNO055 example should use # for the commented out bit. We should only use """ for comments that pylint agrees with.

@evaherrada
Copy link
Collaborator Author

@tannewt @sommersoft Thanks for weighing in. I hadn't thought of it that way, but that definitely makes sense.

@jepler
Copy link
Member

jepler commented Jan 12, 2021

Thank you all for the comments. I think we'll close this one up for now.

@jepler jepler closed this Jan 12, 2021
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