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

Experiment how Furo looks #8420

Closed
wants to merge 1 commit into from
Closed

Experiment how Furo looks #8420

wants to merge 1 commit into from

Conversation

pradyunsg
Copy link
Contributor

@pradyunsg pradyunsg commented Mar 9, 2021

Twitter driven development!

https://twitter.com/pradyunsg/status/1368893934961909763

I never realised that pytest's documentation is a flat list of pages. There's a bunch of quirky things thanks to that, but hey, here's the PR I promised. :)

@RonnyPfannschmidt
Copy link
Member

lovely, we can review on https://pytest--8420.org.readthedocs.build/en/8420/

i'm going to take a closer look once i get back to the computer

thanks!

@pradyunsg
Copy link
Contributor Author

I do have quite a few questions/thoughts, based on how stuff is looking in Furo:

  • Why is there a contents page? Is that to avoid having the entire documentation tree in the index page? That's possible using :hidden: in the toctree directive. Or is it to work around the "display the entire toctree in sidebar" stuff?
  • How do folks feel about breaking the toctree, to give it some separation (similar to how https://pradyunsg.me/furo/ has "Development" in the sidebar)? There seems to be a lot of content that's just in a flat directory with no clear grouping.
  • The titles are... longer than I'd expected. :)

@RonnyPfannschmidt
Copy link
Member

I do have quite a few questions/thoughts, based on how stuff is looking in Furo:

* Why is there a `contents` page? Is that to avoid having the entire documentation tree in the index page? That's possible using `:hidden:` in the toctree directive. Or is it to work around the "display the entire toctree in sidebar" stuff?

i love that suggestion its quite possible we completely missed the opportunity when making this first

* How do folks feel about breaking the toctree, to give it some separation (similar to how https://pradyunsg.me/furo/ has "Development" in the sidebar)? There seems to be a lot of content that's just in a flat directory with no clear grouping.

we want to break/group the documentation since a while now to make it more approachable to different audiences to begin with

* The titles are... longer than I'd expected. :)

i think we are very open for improvements/suggestions there

@gaborbernat
Copy link
Contributor

I'd not do this until pradyunsg/furo#24 is implemented, because it forces a dark color on me, that somehow is harder to read than the light alternative or light one used today...

@pradyunsg
Copy link
Contributor Author

Awesome, thanks for that context @RonnyPfannschmidt!

I've got my hands full with the pip documentation rewrite right now and sphinx theme stuff, but I think a month or so from now, I can come back to this and chip away at this.

If someone else wants to pick this up in the mean time, please feel free to. Or, if it's May and I haven't responded here in a while, please feel free to ping me. :)

@RonnyPfannschmidt
Copy link
Member

Thanks for the details, we got someone that also wants to chip at the docs and we'll coordinate that

@nicoddemus
Copy link
Member

Thanks @pradyunsg, we appreciate the contribution!

I wouldn't worry too much about restructuring things, as @RonnyPfannschmidt mentioned, other people coordinated with us in the mailing list about the docs, so we expect some work to be done in that front soon.

About the theme itself:

I like the overall look of the documentation, congratulations on that. I also particularly like the table of contents added automatically on the right side of each page.

However I noticed that type annotations have the same issues mentioned in that twitter thread:

https://pytest--8420.org.readthedocs.build/en/8420/reference.html#pytest-fixture

image

But I'm not sure if that's something fixable by the theme and/or if you wanted to tackle that in this PR.

Thanks again!

Base automatically changed from master to main March 9, 2021 20:40
@pradyunsg
Copy link
Contributor Author

pradyunsg commented Mar 13, 2021

Noting for future me: @evildmp is doing the (thankless) work of restructuring the documentation! 🌈 (see #8431 for example)

Thank you! ^.^

@pradyunsg
Copy link
Contributor Author

pradyunsg commented Mar 13, 2021

But I'm not sure if that's something fixable by the theme and/or if you wanted to tackle that in this PR.

It might be because I took this PR from a commit before the one that moved the type annotations to the parameter listing.

Realistically though, I don't think the theme can do anything about this - it's what Sphinx is generating that's suboptimal. 🙈

Edit: oh, nvm. #8407 never got merged. That's what ll "fix" this issue.

@evildmp
Copy link
Contributor

evildmp commented Mar 16, 2021

@pradyunsg @nicoddemus Is the issue resolved now? It's not clear to me what it is.

@pradyunsg
Copy link
Contributor Author

Assuming #8455 merges by then, I'll rebase this PR today after my work day! :)

@pradyunsg pradyunsg marked this pull request as draft April 22, 2021 09:04
@pradyunsg
Copy link
Contributor Author

Heyo, are folks interested in this? If so, I'll pick this back up -- otherwise let's close this?

@evildmp
Copy link
Contributor

evildmp commented Mar 18, 2022

Hi @pradyunsg I am. Not my decision, but a) I think it would be a good idea, and b) I think that the adoption of Furo would further help expose issues and opportunities for improvement in the pytest documentation.

@RonnyPfannschmidt
Copy link
Member

@pradyunsg i beleive both @nicoddemus and me would lve to see this land

im a bit swamped so it was off my radar, thanks for the ping

@nicoddemus
Copy link
Member

Yeah it would be interesting for this to be rebased and see how it looks updated. 👍

@pradyunsg
Copy link
Contributor Author

Neato. Lemme file a new PR for this then! :)

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