#19524 closed enhancement (fixed)
fixing a few bad trac roles, and adding some
Reported by:  chapoton  Owned by:  

Priority:  major  Milestone:  sage6.10 
Component:  documentation  Keywords:  trac role 
Cc:  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Jori Mäntysalo, Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  public/19524 (Commits, GitHub, GitLab)  Commit:  f97e522fd09a046ad733045194512f4b82c820f0 
Dependencies:  Stopgaps: 
Description
adding some of the missing links to trac, and correct a few broken ones
Change History (15)
comment:1 Changed 6 years ago by
 Branch set to public/19524
 Commit set to f5c84d1188d3a3e6b06d0e9a372a65eb1eb31342
 Status changed from new to needs_review
comment:2 Changed 6 years ago by
 Reviewers set to Jori Mäntysalo
Seems good.
However, is there reason to use formal trac reference in comments? See for example modular/dims.py
. You have also removed some dot  intentionally? "See trac 1234" or "See trac 1234."  I don't know which one it should be.
comment:3 Changed 6 years ago by
Well, I agree that doing that in the comments is not useful, but not harmful either. It will simplify the grep in the future by removing false positives.
comment:4 Changed 6 years ago by
 Status changed from needs_review to positive_review
OK, good to go then.
comment:5 Changed 6 years ago by
 Status changed from positive_review to needs_work
This shouldn't be done in code:

src/sage/calculus/functions.py
diff git a/src/sage/calculus/functions.py b/src/sage/calculus/functions.py index 29482d9..3048347 100644
a b def wronskian(*args): 94 94 row = lambda n: [diff(f, n) for f in fs] 95 95 # NOTE: I rewrote the below as two lines to avoid a possible subtle 96 96 # memory management problem on some platforms (only VMware as far 97 # as we know?). See trac #2990.97 # as we know?). See :trac:`2990`. 98 98 # There may still be a real problem that this is just hiding for now. 99 99 A = matrix([row(_) for _ in range(len(fs))]) 100 100 return A.determinant()
comment:6 followup: ↓ 7 Changed 6 years ago by
Sorry, but what do you mean ? Do you want to go back to trac #2990
in the comments ?
comment:7 in reply to: ↑ 6 Changed 6 years ago by
Replying to chapoton:
Sorry, but what do you mean ? Do you want to go back to
trac #2990
in the comments ?
Of course. It's ridiculous to put ReST markup outside of docstrings.
comment:8 Changed 6 years ago by
 Commit changed from f5c84d1188d3a3e6b06d0e9a372a65eb1eb31342 to f97e522fd09a046ad733045194512f4b82c820f0
comment:10 Changed 6 years ago by
 Reviewers changed from Jori Mäntysalo to Jori Mäntysalo, Jeroen Demeyer
Impressive.
comment:11 Changed 6 years ago by
This is OK for me.
comment:12 followup: ↓ 13 Changed 6 years ago by
Did somebody check that the documentation actually builds? Apart from this: positive review.
comment:13 in reply to: ↑ 12 Changed 6 years ago by
 Status changed from needs_review to positive_review
Replying to jdemeyer:
Did somebody check that the documentation actually builds? Apart from this: positive review.
I built html docs. And to be sure, I am now (again) running short doctests just to be sure.
I did not check every one of built html page, just some random sample. Those were good. I mark this as positive review.
comment:14 Changed 6 years ago by
 Resolution set to fixed
 Status changed from positive_review to closed
This ticket is closed. The most recent commits that were made after the positive review are not in the release, so open a new ticket for that.
comment:15 Changed 6 years ago by
See #19534.
New commits:
correcting a few bad trac roles
more trac roles