8000 Follow PEP-8 guidelines in tutorial for standard library by bkline · Pull Request #25288 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

Follow PEP-8 guidelines in tutorial for standard library #25288

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Follow PEP-8 guidelines in tutorial for standard library #25288

wants to merge 1 commit into from

Conversation

bkline
Copy link
Contributor
@bkline bkline commented Apr 8, 2021

According to PEP-8, "Arguments on first line forbidden when not using vertical alignment."

No ticket created in b.p.o., as this is a trivial documentation change.

My understanding from reading other tickets related to PEP-8 compliance in the documentation (for example, https://bugs.python.org/issue26030) is that PRs should be submitted as users come across violations of the PEP. If my understanding is wrong, please let me know.

@bkline
Follow PEP-8 guidelines in tutorial for standard library
4fc419c 
According to PEP-8, "Arguments on first line forbidden when not
using vertical alignment."
@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@bkline

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels Apr 8, 2021
@bkline
Copy link
Contributor Author
bkline commented Apr 8, 2021

I have added my GitHub login name to my b.p.o. profile (though I'm not sure why it bumped me to the password confirmation field after submitting the change). I have also signed the CLA, but maybe the bots haven't caught up with each other.

@bkline
Copy link
Contributor Author
bkline commented Apr 9, 2021 •
edited
Loading

Some questions:

  1. The checks complain that "skip issue" and "skip news" labels are missing. How do I add labels?
  2. I have signed the CLA. How do I get the "CLA not signed" label removed? (I followed the link above to check whether the CLA has been received and when I entered my email address I got the error message "500 Internal Server Error
    Server got itself in trouble." It's not clear whether the b.p.o. form wants my email address instead of my GitHub user name so I tried it both ways, and in both cases https://check-python-cla.herokuapp.com/ came back with the same 500 error.)     [I filed a bug report for the Heroku CLA check failures. The maintainer of the service fixed the problem and it's working now, and apparently that satisfied The Knights Who Say Ni bot sufficiently that the CLA not signed label got replaced.]
  3. What does it mean for the checks to report that required checks have been skipped?
  4. I have examined the logs from the "Docs PR - Build documentation" pre-build check. What does this error mean, and what was I supposed to have done to avoid it?
2021-04-08T22:28:01.3554327Z Warning, treated as error:
2021-04-08T22:28:01.3556468Z /home/vsts/work/1/s/Doc/using/configure.rst:416:unknown option: --enable-universalsdk
2021-04-08T22:28:03.8447865Z Makefile:49: recipe for target 'build' failed
2021-04-08T22:28:03.8481903Z make: *** [build] Error 2
2021-04-08T22:28:03.8538418Z ##[error]Bash exited with code '2'.

@willingc
Copy link
Contributor

Hi @bkline,

Thanks for submitting the PR and signing the CLA. I'm going to close this PR instead of merging.

A change for PEP-8 is not needed in this particular case since the change adds an extra assignment that is not necessary in this particular case. In general, adding an extra statement as suggested here is unneeded for ArgumentParser:

    description = 'Show top lines from each file'
    parser = argparse.ArgumentParser(prog='top', description=description)

@willingc willingc closed this May 14, 2021
@bkline
Copy link
Contributor Author
bkline commented May 14, 2021

Well, "should the tutorial follow PEP-8?" (yes) and "is it necessary to add another assignment to do so?" (no) are two different questions. I'll open a new PR which satisfies the goal of complying with PEP-8 without violating the unwritten constraint that we not add another assignment.

@bkline
Copy link
Contributor Author
bkline commented May 14, 2021

I would appreciate answers to the questions I asked above on April 9. Thanks!

@bkline
Copy link
Contributor Author
bkline commented May 14, 2021

Replacement PR: #26127

@JelleZijlstra
Copy link
Member
JelleZijlstra commented Apr 3, 2022 •
edited
Loading

Hey @bkline, I'm sorry that your questions here weren't answered, and that your PR lingered for so long. Here are some belated answers:

  1. Labels can only be added by core developers and triagers. In the future, you can put a note on the issue and hopefully a triager or core dev will come along and apply the label.
  2. skipped
  3. Not sure when this happens exactly. I've seen a few instances of it, and merging main into the PR branch tends to be enough to get required checks to pass.
  4. Not familiar with that error. Occasionally bugs creep onto main for various reasons, either bad merges or changes in a dependency. Or it could just be a spurious failure. In general, you can search for open PRs or bugs.python.org issues addressing the problem, and when it's fixed merge main into your branch to get CI to pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
awaiting review docs Documentation in the Doc dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bkline @the-knights-who-say-ni @willingc @JelleZijlstra @bedevere-bot
0