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

Fix docstring syntax errors #1312

Merged
merged 7 commits into from
Jun 22, 2020
Merged

Fix docstring syntax errors #1312

merged 7 commits into from
Jun 22, 2020

Conversation

mthuurne
Copy link
Contributor

@mthuurne mthuurne commented Jun 21, 2020

This removes some noise from the pydoctor logging, so actual problems will stand out more.

Contributor Checklist:

Since docstrings are strings, a '\n' will insert a newline. If the
documentation should read '\n', the backslash should be escaped.
The epytext syntax requires a list to be indented.
Otherwise the epytext parser will complain about the indentation.
@mthuurne
Copy link
Contributor Author

mthuurne commented Jun 21, 2020

CircleCI refuses to let me view the details of the test failure unless I hand it read/write access to all my repositories, which I'm obviously not going to agree to. Is this a bug? If not, I think it's a completely unacceptable policy, both in terms of privacy and security.

Update: exarkun was kind enough to copy-paste the error message on IRC; it was about the missing news fragment. I still think it's a problem though that CircleCI won't show details unless you hand it a bunch of permissions it doesn't need.

@mthuurne
Copy link
Contributor Author

mthuurne commented Jun 21, 2020

The problem identified by Travis is the one I mentioned in the initial comment: lint errors near, but not caused by, the changed lines. Should I fix those even though they have nothing to do with the actual purpose of the PR?

I fixed the unrelated lint errors in a separate commit.

These are in close proximity to, but not actually part of, the changed
docstrings.
Copy link
Contributor

@rodrigc rodrigc left a comment

Choose a reason for hiding this comment

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

This looks OK

@mthuurne mthuurne merged commit 81a80f1 into trunk Jun 22, 2020
@mthuurne mthuurne deleted the 9868-mthuurne-baddocstring branch June 22, 2020 07:03
@rodrigc
Copy link
Contributor

rodrigc commented Jun 22, 2020

@mthuurne in future when you merge your PR to Twisted's trunk, you need to manually add these three lines to the merge commit message:

Author:
Reviewer:
Fixes: ticket:

This auto-closes the Trac ticket, and updates https://twistedmatrix.com/highscores

See: https://twistedmatrix.com/trac/wiki/ReviewProcess#Authors:Howtomergethechangetotrunk

You can also look at some other merge commits in Twisted for examples.

@mthuurne
Copy link
Contributor Author

Sorry, this was my first time merging a commit for Twisted; I'll do that next time.

@rodrigc
Copy link
Contributor

rodrigc commented Jun 22, 2020

It's OK, the Twisted dev process is a bit complicated for newcomers, but once you get the hang of it, it is OK.

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.

None yet

2 participants