Replace all :trac: with :issue: in docstrings #37390
Conversation
|
Perhaps not to do in this PR, but I was thinking of changing "github issue" to just "issue". For example, is changed to We may redefine |
No problem (if we could avoid merge conflicts). |
kwankyu
added
p: minor / 4
s: positive review
and removed
p: trivial / 5
s: needs review
labels
|
re:
Yeah, this could be done. Although there's probably a non-trivial amount of people who do not know what an "issue" is and "GitHub issue" might help newcomers better understand the docstrings from within Sage? |
|
In the trac era, it was "trac #7797". I liked it :-) |
|
I have no strong opinions. Whatever is decided we can do in a new PR? |
I am collecting opinions. If a few collected opinions are supportive for the change, yes we can do it in a new PR. If not clearly supportive, a poll may be necessary on sage-devel. Thanks for your opinion. |
|
Please don't do "Github issue". It is a lot of text with extremely little extra value. I would hope that non-developers do not see such notices unless they are using deprecated functionality. In this case it comes with a complete link. |
|
Yes, I also prefer "Issue #7797" (capitalized) |
|
merge conflict |
|
@vbraun This PR is very likely to incur merge conflicts. So could you consider making a beta only for this PR? That is, you release next beta, and then we rebase this PR to the beta, and then you immediately merge this PR and release a new beta. |
|
This will fix the merge conflicts: diff --cc src/sage/rings/complex_arb.pyx
index 4ee2ec45015,31d1baf0f38..46c01da4eb6
--- a/src/sage/rings/complex_arb.pyx
+++ b/src/sage/rings/complex_arb.pyx
@@@ -4792,18 -4792,14 +4792,14 @@@ cdef class ComplexBall(RingElement)
sage: n = CBF(1,1)
sage: m = CBF(-2/3, 3/5)
- sage: n.elliptic_pi_inc(CBF.pi()/2, m) # arb216
- [0.8934793755173 +/- ...e-14] + [0.95707868710750 +/- ...e-15]*I
- sage: n.elliptic_pi_inc(CBF.pi()/2, m) # arb218 - this is a regression, see :issue:28623
- sage: n.elliptic_pi_inc(CBF.pi()/2, m) # this is a regression, see :trac:28623
++ sage: n.elliptic_pi_inc(CBF.pi()/2, m) # this is a regression, see :issue:28623
nan + nan*I
sage: n.elliptic_pi(m)
[0.8934793755173...] + [0.957078687107...]*I
sage: n = CBF(2, 3/7)
sage: m = CBF(-1/3, 2/9)
- sage: n.elliptic_pi_inc(CBF.pi()/2, m) # arb216
- [0.2969588746419 +/- ...e-14] + [1.3188795332738 +/- ...e-14]*I
- sage: n.elliptic_pi_inc(CBF.pi()/2, m) # arb218 - this is a regression, see :issue:28623
- sage: n.elliptic_pi_inc(CBF.pi()/2, m) # this is a regression, see :trac:28623
++ sage: n.elliptic_pi_inc(CBF.pi()/2, m) # this is a regression, see :issue:28623
nan + nan*I
sage: n.elliptic_pi(m)
[0.296958874641...] + [1.318879533273...]*I |
|
@vbraun Please merge this to 10.3.rc3. |
|
Documentation preview for this PR (built with commit ad8fe87; changes) is ready! 🎉 |
Following the PR sagemath#37385, I have replaced all occurrences of `:trac:` with `:issue:`. If changing so much in one PR is wrong and this should be done file by file while working on other functions feel free to close this PR. URL: sagemath#37390 Reported by: Giacomo Pope Reviewer(s): Giacomo Pope, Kwankyu Lee
vbraun
closed this pull request by merging all changes
into
sagemath:develop
in
e417e22
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> As discussed in sagemath#37390 (comment), we change how Sphinx role ``:issue:`` is rendered. For example, short ``` By Issue sagemath#7797, there is a different implementation ... ``` instead of current ``` By github issue sagemath#7797, there is a different implementation ... ``` Arguments for the short form are > Please don't do "Github issue". It is a lot of text with extremely little extra value. > In the trac era, it was "trac sagemath#7797". <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#37403 Reported by: Kwankyu Lee Reviewer(s): Travis Scrimshaw
Following the PR #37385, I have replaced all occurrences of
:trac:with:issue:.If changing so much in one PR is wrong and this should be done file by file while working on other functions feel free to close this PR.