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

"Edit" link on readthedocs documentation pages links to http 404 page #505

Closed
michaelkaye opened this issue May 2, 2023 · 8 comments · Fixed by #596
Closed

"Edit" link on readthedocs documentation pages links to http 404 page #505

michaelkaye opened this issue May 2, 2023 · 8 comments · Fixed by #596
Labels

Comments

@michaelkaye
Copy link
Contributor

For example:

https://towncrier.readthedocs.io/en/stable/index.html

has a pencil icon at the top, that normally implies "edit these docs", but currently points to

https://github.com/twisted/towncrier/edit/b0e201f8d84819f0e5604e0cb029fb3b8b4cf2df/docs/index.rst

Which is currently a github 404 page.

Making this work lowers the barrier to entry for simple documentation improvements (eg #504 was made harder by this)

@adiroiban
Copy link
Member

Thanks Michael for the report.

I think the issue here is that we generate the edit link based on the PR branch head.

The PR has a branch made by Github on the fly by merging the branch associated with the PR owth the target branch.

For the release documentation, I think that we should generate the edit link based on a tag


We need someone to take a look into our Sphinx configuration and find a fix for this

@adiroiban adiroiban added the docs label May 2, 2023
@hynek
Copy link
Member

hynek commented May 4, 2023

FWIW it looks like most projects have broken edit buttons. Brief googling only unearths workarounds to hide the button. 😳

@adiroiban
Copy link
Member

I think that the
READTHEDOCS_VERSION_NAME can help.

I think the issue is with the furo theme, that lacks support for Read The Docs build environmnet.


My comments about GitHub Actions were wrong.

We are not building the docs on GitHub.

The documentation is generated and distributed via the Read The Docs servers and CDN.


I can see that furo has a configuratio for this - https://furo.readthedocs.io/customisation/edit-button.html#with-popular-vcs-hosts

so inside conf.py I think that we need something like

html_theme_options = {
    "source_repository": "https://github.com/pradyunsg/furo/",
-    "source_branch": "main",
+    "source_branch": os.environ.get('READTHEDOCS_VERSION_NAME', 'main'),
    "source_directory": "docs/",
}

@hynek
Copy link
Member

hynek commented May 7, 2023

Are you sure you want the version? That begets edits on a potentially outdated document.

I think it should always be against trunk. Best case the editors learns that the change is already made.

@adiroiban
Copy link
Member

Are you sure you want the version? That begets edits on a potentially outdated document.

Good question.

So I guess that is best to just disable the edit button by default

html_theme_options = {
    "top_of_page_button": No,
}

and enable the edit button only for the latest version

html_theme_options = {
    "top_of_page_button": "edit",
    "source_repository": "https://github.com/twisted/towncrier/",
    "source_branch": "trunk",
    "source_directory": "docs/",
}


If you look at an older version, and then click edit, and then you land on the latest version, things might be confusing.

@michaelkaye
Copy link
Contributor Author

and enable the edit button only for the latest version

When you say "latest" here do you mean the latest stable documentation linked from the project README, or do you mean the latest or trunk version?

Why not take the 'confusing' route for a bit; you'll get direct feedback by the documentation PRs that turn up if there is confusion going on. Worst case you tweak and rebuild the docs and the button goes away again?

I'm not part of your project; but I think i'm the target of this edit button - don't hide the edit button at all, if you want people to fix typos like the one that triggered this.

@adiroiban
Copy link
Member

Thanks @michaelkaye for the feedback.

I think that link is useful and making it easy to fix typos is a good cause that it's worth fighting for :)

Why I say latest, I mean trunk - https://towncrier.readthedocs.io/en/latest/

At this point, all changes should be done on the trunk branch.

So if you are on this page https://towncrier.readthedocs.io/en/22.12.0/tutorial.html and find a typo, you should check whether the typo is still in trunk and then send a fix for that version.

@SmileyChris
Copy link
Contributor

Check out my PR, I think it should be good enough

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants