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

Fix the 'edit on github' link for stable versions of a projet #2428

Conversation

magopian
Copy link

Fixes #1820

This is a naive approach, and might break things, for example for other VCS
than GitHub. Feedback welcome!

The rationale here is that the identifier of the stable build is the hash
of the tag, and not of a commit. It thus leads to a 404 when building the link
to edit on GitHub (see the comment in the original
issue
).
If instead of storing the tag hash in the identifier we stored the
verbose_name of the original branch (its name), it would be used to build the
link (just the same way we use the verbose name for tags, and not commit
hashes).

@techtonik
Copy link
Contributor

techtonik commented Nov 9, 2016

Is it possible to get test for that?

I am afraid I need more complete picture on how readthedocs works with tags, branches and slugs.

@magopian
Copy link
Author

Sure, any tips on where I should put it? readthedocs/rtd_tests/tests/test_sync_versions.py maybe?

@leplatrem
Copy link

@magopian let's assume test_sync_versions.py is the right location and we move on :)

@berkerpeksag
Copy link
Member

Yes, readthedocs/rtd_tests/tests/test_sync_versions.py is a good place to put the test. There is already a test for the update_stable_version method so some parts of it can be reused :)

Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've looked at this, and it just seems to change the identifier for the stable version on creation. If we want to fix this, it will need to be back ported to all projects with a migration.

Another way to fix this is with logic on the display, when we have a stable branch, link to some other path on the repo.

@ericholscher
Copy link
Member

The link in the footer is actually doing the correct thing (as you can see here: http://mockingjay.readthedocs.io/en/stable/) -- so we just need to fix the variable passed to the button.

@ericholscher
Copy link
Member

Actually looks like this has been fixed, so closing this PR :)

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