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

Insert URL to specific pages #4

Open
humitos opened this issue Jul 25, 2022 · 11 comments
Open

Insert URL to specific pages #4

humitos opened this issue Jul 25, 2022 · 11 comments

Comments

@humitos
Copy link
Member

humitos commented Jul 25, 2022

We could detect there were changes to specific files (via #3) and insert direct links to those documentation pages as well. For example, if the file docs/page/section.rst was modified, we could insert the link https://readthedocs-preview--1.org.readthedocs.build/en/1/pages/section.html

There are some notes to mention:

  • we need to know where is the root directory that matches the root of the URL (in this case, we need to remove docs/ from the filename)
  • the URL is a pretty URL (/pages/section/ or /pages/section.html)
  • support user-configurable amount of links (with a small default), to avoid flooding in case of many files changed
@benjaoming
Copy link

It will be super-useful to have a list of <Section title>: <URL>

Also to handle the special case: No changes in docs. Most PRs (unfortunately) don't contain documentation changes. This will be interesting to explore in the future. If, however, there is automated API docs, we might have a situation where almost all PRs generate really cool and meaningful links.

@humitos
Copy link
Member Author

humitos commented Jul 28, 2022

It will be super-useful to have a list of <Section title>: <URL>

Getting the title correctly may be a little harder to do. However, I'd assume we can get the h1 from each of the modified files/pages and use that. However, this "technique" won't work for doctools like Docsify 🙃

Also to handle the special case: No changes in docs. Most PRs (unfortunately) don't contain documentation changes. This will be interesting to explore in the future. If, however, there is automated API docs, we might have a situation where almost all PRs generate really cool and meaningful links.

I'm not sure to follow this. Can you expand this and give an explicit example of how it would work, please?

@benjaoming
Copy link

I'm not sure to follow this. Can you expand this and give an explicit example of how it would work, please?

So these are more scenarios of what can happen with the list:

Scenario 1: Some documentation pages got updated (the intended scenario)

User modified or added a .md or .rst file: We create a nice little list corresponding to the pages that have been changed such that the user may click directly to the section of the docs that were modified/added.

Scenario 2: No changes! (or everything changed or we have no idea)

We have built the docs again but actually we don't have any changes to list? Perhaps no files were edited and the documentation is completely unchanged, so we shouldn't "disturb" the PR reader with links? Perhaps a configuration file was edited but nothing detectable changed? Perhaps the footer was changed and everything is updated?

Scenario 3: Everything changed!

We detect that all pages have changed, perhaps because we correctly see that the footer or configuration or navigation structure is affected.

Scenario 4: API docs changed

Nothing in the documentation pages have changed, but the automatically generated API docs have changes.

@humitos
Copy link
Member Author

humitos commented Aug 1, 2022

@benjaoming it seems you are describing exactly what we have discussed already in readthedocs/readthedocs.org#8545

This problem is pretty complex to solve correctly for all the users under all these scenarios and that's why we haven't built it yet. Due to its complexity, I do not want to solve this problem on this simple action itself. I think it would be enough for most cases if we can just put the links for all the pages matching documentation files modified in the PR.

People will always have the link to the index itself in case they want to dig more on the other pages that may have been modified because of a code change, for example.

@choldgraf
Copy link

Just noting that:

  • this would definitely be useful
  • I agree that "just give a list of the URLs for changed pages" would be 80% of the way there and an excellent first step
  • you might put that list in a "details" block if it gets too long

@benjaoming
Copy link

@humitos I believe that conditional builds are different from this issue because here, we are trying to detect the changes post-build while #8545 tries to detect changes pre-build.

@humitos
Copy link
Member Author

humitos commented Aug 1, 2022

@benjaoming

we are trying to detect the changes post-build while #8545 tries to detect changes pre-build.

This is not exactly how the GitHub Action works now, tho. We could execute the action only after the build has finished and succeeded. However, in that case, I'm not sure what extra information will we have available in that case currently: there is no API that exposes what files have changed in the documentation output or similar. How are you thinking that post-build will give us more useful data here?

@benjaoming
Copy link

Sorry I should have presented that (wrong) assumption earlier. In this case, it's much more like #8545. I like the idea of solving these two issues at the same time, both predicting when to build and what URLs will change. There's a lot of developers'+server time to be won if we can get our predictions right in >90% of cases.

@pradyunsg
Copy link

Hi hi! This would be useful for Python PEPs repository as well as the Python devguide, where this would make the reviewer process easier by eliminating the need to navigate to the specific PEP/page that has been modified in the PR!

If this is something where all the information needed to detemine the relevant URL is already available (or something you'd be OK having explicit configuration for), that would be useful to know since someone (maybe me) from the community could contribute the logic change for that. :)

@humitos
Copy link
Member Author

humitos commented Oct 24, 2023

@pradyunsg

If this is something where all the information needed to detemine the relevant URL is already available (or something you'd be OK having explicit configuration for)

Some of the relevant information is present, but some of it has to be explicitly defined by the user setting up the GHA I'd say. GHA provides the files that were modified in the repository (e.g. using github.rest.pulls.listFiles inside the action) but we will need to know "what's the logic to convert that file into a URL".

That logic will depend on the doctool used (MkDocs, Sphinx, Docusaurus, etc) and also on the options specified in the doctool itself (e.g. "pretty URL" --ending with .html or not). I think we can have these logics hardcoded in the GHA itself and ask the user for the missing pieces of configurations so we can effectively generate the URL based on the filename. It may not work for all the cases, but covering the most common ones would probably be enough for the first version 😄

I don't think I will have the time to prioritize this work sooner, but I'd be happy to help with reviews 🧐 and directions ⤴️ if someone wants to jump into this.

@humitos
Copy link
Member Author

humitos commented Jul 31, 2024

We start talking about a potential implementation for this feature at readthedocs/readthedocs.org#11507. I'd love to have your feedback.

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

No branches or pull requests

4 participants