Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#19524 closed enhancement (fixed)

fixing a few bad trac roles, and adding some

Reported by: chapoton Owned by:
Priority: major Milestone: sage-6.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) 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 4 years ago by chapoton

  • Branch set to public/19524
  • Commit set to f5c84d1188d3a3e6b06d0e9a372a65eb1eb31342
  • Status changed from new to needs_review

New commits:

bcef7f3correcting a few bad trac roles
f5c84d1more trac roles

comment:2 Changed 4 years ago by jmantysalo

  • 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 4 years ago by chapoton

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 4 years ago by jmantysalo

  • Status changed from needs_review to positive_review

OK, good to go then.

comment:5 Changed 4 years ago by jdemeyer

  • 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): 
    9494            row = lambda n: [diff(f, n) for f in fs]
    9595        # NOTE: I rewrote the below as two lines to avoid a possible subtle
    9696        # memory management problem on some platforms (only VMware as far
    97         # as we know?).  See trac #2990.
     97        # as we know?).  See :trac:`2990`.
    9898        # There may still be a real problem that this is just hiding for now.
    9999        A = matrix([row(_) for _ in range(len(fs))])
    100100        return A.determinant()

comment:6 follow-up: Changed 4 years ago by chapoton

Sorry, but what do you mean ? Do you want to go back to trac #2990 in the comments ?

Last edited 4 years ago by chapoton (previous) (diff)

comment:7 in reply to: ↑ 6 Changed 4 years ago by jdemeyer

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 4 years ago by git

  • Commit changed from f5c84d1188d3a3e6b06d0e9a372a65eb1eb31342 to f97e522fd09a046ad733045194512f4b82c820f0

Branch pushed to git repo; I updated commit sha1. New commits:

9beea8dremoving useless trac role rest syntax in comments
f97e522removing trac roles in comments in pyx files

comment:9 Changed 4 years ago by chapoton

  • Status changed from needs_work to needs_review

ok, done

comment:10 Changed 4 years ago by jdemeyer

  • Reviewers changed from Jori Mäntysalo to Jori Mäntysalo, Jeroen Demeyer

Impressive.

comment:11 Changed 4 years ago by jmantysalo

This is OK for me.

comment:12 follow-up: Changed 4 years ago by jdemeyer

Did somebody check that the documentation actually builds? Apart from this: positive review.

comment:13 in reply to: ↑ 12 Changed 4 years ago by jmantysalo

  • 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 4 years ago by vbraun

  • 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 4 years ago by jdemeyer

See #19534.

Note: See TracTickets for help on using tickets.