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

Make "[source]" link to develop branch on github #38121

Merged
merged 2 commits into from
Jun 1, 2024

Conversation

kwankyu
Copy link
Collaborator

@kwankyu kwankyu commented May 31, 2024

This is a bug in #37589. I made other small edits.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@kwankyu kwankyu changed the title source link to develop version Make "[source]" link to develop version on github May 31, 2024
@kwankyu kwankyu changed the title Make "[source]" link to develop version on github Make "[source]" link to develop branch on github May 31, 2024
Copy link

Documentation preview for this PR (built with commit 1d0343e; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@kwankyu kwankyu requested a review from mkoeppe May 31, 2024 18:45
@mkoeppe
Copy link
Contributor

mkoeppe commented May 31, 2024

So this simply completes the change that you said you'd do in #37589 (comment)?

@mkoeppe
Copy link
Contributor

mkoeppe commented May 31, 2024

By the way, do you think this would this be easy to do:

@kwankyu
Copy link
Collaborator Author

kwankyu commented May 31, 2024

So this simply completes the change that you said you'd do in #37589 (comment)?

If you mean Problem 2 mentioned in the comment, no. This PR fixes a bug so that [source] opens a github page that allows me to edit the source file (previously not worked because the github page was on a tag branch). But still github does not allow me to commit the changes because I am in sagemath/sage, not on my forked repo.

@mkoeppe
Copy link
Contributor

mkoeppe commented May 31, 2024

Thanks for the explanation. LGTM.

@kwankyu
Copy link
Collaborator Author

kwankyu commented May 31, 2024

ideally the banner at the top would give a link to the PR instead of saying that it's for the development version.

For that, the PR number should be available to the doc build process. Perhaps the PR number could be added to the version...

But I really don't understand how such a link could be useful... The user typically comes to the doc from the PR.

@kwankyu
Copy link
Collaborator Author

kwankyu commented May 31, 2024

... But still github does not allow me to commit the changes because I am in sagemath/sage, not on my forked repo.

A manual way to be able to commit is (of course) to patch the url (from "sagemath/sage" to "user/sage" :-) before starting to edit.

@mkoeppe
Copy link
Contributor

mkoeppe commented May 31, 2024

But I really don't understand how such a link could be useful... The user typically comes to the doc from the PR.

Consider the scenario of users who tend to have too many tabs open in the browser

@kwankyu
Copy link
Collaborator Author

kwankyu commented May 31, 2024

I see. I can also imagine that one opens a doc tab separate from a PR tab, which I often do. In that case, back button does not go to the PR.

@kwankyu
Copy link
Collaborator Author

kwankyu commented May 31, 2024

[#37759] Also the source links should link to the branch of the PR, not the tagged version.

No idea.

@mkoeppe
Copy link
Contributor

mkoeppe commented May 31, 2024

I've added a link to #37759 to explain where this information is available

@kwankyu
Copy link
Collaborator Author

kwankyu commented May 31, 2024

Thanks. Thanks for review!

@vbraun vbraun merged commit 973a7e3 into sagemath:develop Jun 1, 2024
23 checks passed
@mkoeppe mkoeppe added this to the sage-10.4 milestone Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants