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

Broken edit links #1637

Closed
bdarnell opened this issue Sep 9, 2015 · 17 comments · Fixed by #3302
Closed

Broken edit links #1637

bdarnell opened this issue Sep 9, 2015 · 17 comments · Fixed by #3302
Labels
Bug A bug

Comments

@bdarnell
Copy link

bdarnell commented Sep 9, 2015

Two issues which may or may not be related:

The "latest" branch works fine (linking to "master"); this is only a problem for pages based on other branches (migrated from readthedocs/sphinx_rtd_theme#236)

@gregmuellegger
Copy link
Contributor

Thanks for the report! Indeed these are two distinct issues. The first will be addressed with #1643.

I also had a look at the second, but it's a little tricky. The reason is that stable usually is a special version name on our end. We alias the branch or tag of a project that is a valid version number (like v1.3.4) and the highest version number of all tags/branches as being the stable version. So that's why this name gets an extra treatment.

The Tornado project seems to suffer from this a little as you have a branch called stable. Now the reason why the link in the footer-popup (clicking v: stable) works is that this part of the page does not check for the special stable name (so actually it's broken for all projects that are not having a stable branch in the project) and the link at the top of the page "Edit on GitHub" is broken for your case as it checks for the special name.

I will try to come up with a fix but currently I think for really making the "stable" special casing work, we might need to refactor how we store the branch/commit identifier information. I'll keep you posted.

@bdarnell
Copy link
Author

Ah, I didn't realize that about stable. I'm using the stable branch to do essentially the same thing by hand, so maybe I can just get rid of my branch. How would I replace my stable branch with an alias for branch4.2 or tag v4.2.1?

@gregmuellegger
Copy link
Contributor

We are trying to figure out the stable branch ourself by parsing all available branch and tag names. The highest valid version number that is compliant to PEP-440 we be set as the stable branch.

@bdarnell
Copy link
Author

So if I just delete my stable branch from github, readthedocs will see that it's gone and replace it with one of my branches or tags?

@gregmuellegger
Copy link
Contributor

The second point of your issue will be addressed with #1650

@gregmuellegger
Copy link
Contributor

@bdarnell Should work this way, yes. Maybe you need to push one commit (should be irrelevant which branch) since I don't know if the deletion of a branch triggers a post commit hook.

@gregmuellegger
Copy link
Contributor

The second point (bad links from the top right "Edit on GitHub" page) will be fixed with the next deploy as we have merged #1650

@techtonik
Copy link
Contributor

So is it fixed?

@humitos
Copy link
Member

humitos commented May 18, 2017

The issue with the "Edit on Github" for auto-generated pages is still present. I just tried it in my local instance of RTD and I can see the link for auto-generated pages. Also, the last issue linked here from another repo is from November 2016.

Regarding the second one and the branches, I'm not really sure to understand it or how to make a good test.

@rawtaz
Copy link

rawtaz commented Feb 14, 2018

@ericholscher This issue is still a problem on the website, the "Edit on GitHub" still doesn't work for the stable version.

I see there has been a bunch of related issues (of which I proposed a very very simple fix for the problem in #1820) and PRs. But the issue persists.

What about the approach I suggested? It seems to be a tough issue to tackle with the approaches tried so far, so perhaps just going back to a KISS fix would enable us to have that Edit button working again? :)

@techtonik
Copy link
Contributor

@rawtaz the first thing that should be done is to ensure prod runs git 2+ #3615, then the rabbit hole will be less deep.

@rawtaz
Copy link

rawtaz commented Feb 15, 2018

@ericholscher Is there something blocking #3615 that seems to be the fix for the long-running Edit button problems?

@rawtaz
Copy link

rawtaz commented Feb 15, 2018

@techtonik Thanks for working on this. I'm still unsure why we don't just make RTD pull in the latest release tag (when available, of course) instead of a stable branch when the latter is missing. Seems like a very simple fix to the problem. Could have been solved a long time ago and without having to upgrade prod etc.

@techtonik
Copy link
Contributor

@rawtaz the problem is in annotated tags only. If release tag is annotated, RTD gets the wrong hash.

@rawtaz
Copy link

rawtaz commented Feb 15, 2018

Hm, I'm not sure we're talking about the same thing :)

What I'm talking about is the fact that when you are on the stable version of a project's docs on RTD, and that project does not have a corresponding stable branch (that is named stable, that is), the Edit on GitHub button fails to identify a proper URL on GitHub to link the button to. It works fine for the master branch (the latest version on RTD), it's just when there's no corresponding stable branch that the button doesn't link properly.

Since RTD does however know about release tags for the project, it can simply use the latest release tag's URL for the edit button, so instead of linking to an improper URL for a non-existing stable branch, it can just link to the state of the master branch at that latest tag.

Let me be super clear by three examples:

  1. The latest version on http://restic.readthedocs.io/en/latest/ has an edit button that goes to https://github.com/restic/restic/blob/master/doc/index.rst - this works.
  2. The version for the last release, v0.8.1 at http://restic.readthedocs.io/en/v0.8.1/, has an edit button that goes to https://github.com/restic/restic/blob/v0.8.1/doc/index.rst - this works too.
  3. The stable version at http://restic.readthedocs.io/en/stable/ has an edit button that goes to https://github.com/restic/restic/blob/84253934823edf6acd2fbb24a7f39ccbd1313617/doc/index.rst - this does NOT work, and from what I've gathered it is because there is no corresponding stable branch in the repository at GitHub.

So, wouldn't a very simple solution to the problem then be to make the edit button for the stable version go to the same URL as the last release (the one in #2 above)? That's what stable is meant to represent anyway, so it could just go straight there, as they'll be the same point in the repo.

I'm sorry if I repeated what you already know, it just sounds to me that we're discussing two different matters/problems.

@humitos
Copy link
Member

humitos commented Feb 16, 2018

  1. The stable version at http://restic.readthedocs.io/en/stable has an edit button that goes to restic/restic:doc/index.rst@8425393 - this does NOT work, and from what I've gathered it is because there is no corresponding stable branch in the repository at GitHub.

This shouldn't be a problem. None of the repositories need to have a stable branch/tag; this is something created by RTD itself.

Take a look at this example that works as it's expected: http://requests-oauthlib.readthedocs.io/en/stable/. If you click in Edit on Github link, it just works.

I don't know why you are having that exact problem yet. We need replication for this in a local installation so it's possible to fix it. Feel free to open a new issue (since this is closed and about a similar problem) with these steps and examples you mentioned here.

@techtonik
Copy link
Contributor

stable is an alias for some other tag that could be annotated and then hash becomes invalid.

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

Successfully merging a pull request may close this issue.

6 participants