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

References overhaul #236

Merged
merged 4 commits into from
Oct 19, 2019
Merged

References overhaul #236

merged 4 commits into from
Oct 19, 2019

Conversation

hoefling
Copy link
Member

Closing #186.

Running linkcheck now yields only one broken link to tgoodlet user account, rest are fixed. Also replaced hardcoded links with intersphinx refs where possible. Extended apidoc to a minimal amount of private classes/methods, enough just to fix the existing refs.

The second commit adds some crossrefs between the docstrings and some minor formatting improvements. This one is more a suggestion and I wouldn't mind throwing it away - check whether it really adds a significant amount of readability.

…here possible

Signed-off-by: Oleg Höfling <oleg.hoefling@gmail.com>
Signed-off-by: Oleg Höfling <oleg.hoefling@gmail.com>
@codecov-io
Copy link

codecov-io commented Sep 15, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #236   +/-   ##
=======================================
  Coverage   92.91%   92.91%           
=======================================
  Files          14       14           
  Lines        1651     1651           
  Branches      115      115           
=======================================
  Hits         1534     1534           
  Misses        101      101           
  Partials       16       16
Impacted Files Coverage Δ
src/pluggy/hooks.py 94.28% <ø> (ø) ⬆️
src/pluggy/manager.py 94.38% <ø> (ø) ⬆️

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 fe5a72b...d5b871d. Read the comment docs.

@goodboy
Copy link
Contributor

goodboy commented Oct 16, 2019

Running linkcheck now yields only one broken link to tgoodlet user account,

@hoefling that's my old account. I guess just link to my new one?

Other then that this looks mostly good to me though I'll do another run through after others have.

Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

Signed-off-by: Oleg Höfling <oleg.hoefling@gmail.com>
Signed-off-by: Oleg Höfling <oleg.hoefling@gmail.com>
@hoefling
Copy link
Member Author

@goodboy Oh, didn't know that - easy fix, then.

Copy link
Contributor

@goodboy goodboy left a comment

Choose a reason for hiding this comment

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

@hoefling wow this looks great!

I actually learned a few more fancy things about sphinx reading through this.
Really nice to see the intersphinx_mapping stuff put to work too - it definitely minimizes on the number of redefined links!

@goodboy
Copy link
Contributor

goodboy commented Oct 19, 2019

@RonnyPfannschmidt @nicoddemus this get's a stamp from me. If you two don't have the time to look through this I think it's ready to land.

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

just awesome, thanks

@goodboy goodboy merged commit 9890e90 into pytest-dev:master Oct 19, 2019
@hoefling
Copy link
Member Author

@goodboy thanks, glad it was helpful. Sphinx offers more fancy stuff that could be used - for example, I noticed definitions throughout the docs that could be collected under a .. glossary:: directive and referenced via :term:, creating nice hyperlinks to the def paragraph (e.g. for host, plugin, hookspec, hookimpl etc). See stdlib glossary for an example. Or referencing Python keywords via :keyword: role, e.g. yield, None, True etc. May be an overkill, though...

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.

5 participants