-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Comments
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 The Tornado project seems to suffer from this a little as you have a branch called 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. |
Ah, I didn't realize that about |
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. |
So if I just delete my |
The second point of your issue will be addressed with #1650 |
@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. |
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 |
So is it fixed? |
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. |
@ericholscher This issue is still a problem on the website, the "Edit on GitHub" still doesn't work for the 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? :) |
@ericholscher Is there something blocking #3615 that seems to be the fix for the long-running Edit button problems? |
@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. |
@rawtaz the problem is in annotated tags only. If release tag is annotated, RTD gets the wrong hash. |
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 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 Let me be super clear by three examples:
So, wouldn't a very simple solution to the problem then be to make the edit button for the I'm sorry if I repeated what you already know, it just sounds to me that we're discussing two different matters/problems. |
This shouldn't be a problem. None of the repositories need to have a 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. |
|
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)
The text was updated successfully, but these errors were encountered: