Opened 5 years ago
Closed 4 years ago
#22604 closed enhancement (fixed)
autodoc unforking again
Reported by:  jdemeyer  Owned by:  

Priority:  major  Milestone:  sage8.2 
Component:  build  Keywords:  
Cc:  hivert, embray  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Erik Bray 
Report Upstream:  N/A  Work issues:  
Branch:  14b02b0 (Commits, GitHub, GitLab)  Commit:  14b02b01c572840e1c7958a80a397af840f7b1ed 
Dependencies:  Stopgaps: 
Description
Move some changes from upstream Sphinx autodoc.py
to sage_autodoc.py
Change History (14)
comment:1 Changed 5 years ago by
 Dependencies set to #22601
comment:2 Changed 5 years ago by
 Branch set to u/jdemeyer/ticket/22604
comment:3 Changed 5 years ago by
 Commit set to 130be6d92070befcf662862c1900156656b5f1ec
 Status changed from new to needs_review
comment:4 Changed 4 years ago by
 Cc embray added
 Dependencies #22601 deleted
 Milestone changed from sage7.6 to sage8.2
comment:5 followup: ↓ 6 Changed 4 years ago by
I'm not exactly clear on what this is doing. Is sage_autodoc a copy of the autodoc module from Sphinx with some patches applied? Is the point here just to reapply those patches on top of a newer copy of the module?
(I assume the goal is to eventually do away with the copy entirely...?)
comment:6 in reply to: ↑ 5 Changed 4 years ago by
Replying to embray:
Is sage_autodoc a copy of the autodoc module from Sphinx with some patches applied?
Yes.
Is the point here just to reapply those patches on top of a newer copy of the module?
No. The point is to reduce the patching, i.e. reduce the diff between the real autodoc and the Sage autodoc.
Ideally, we should eventually reduce the diff to zero (by fixing Sage and/or Sphinx).
comment:7 Changed 4 years ago by
Well reduce, sure, but it seems like some of both. That is, you started with the new upstream version, reapplied any patches from Sage, sans patches that are no longer necessary.
comment:8 followups: ↓ 9 ↓ 10 Changed 4 years ago by
It might be nice if there were a comment specifying exactly what version of the upstream code this is based on (e.g. with a link to GitHub at the correct revision). That way it would be easy enough to compare to the original to see what the differences are.
comment:9 in reply to: ↑ 8 Changed 4 years ago by
Replying to embray:
It might be nice if there were a comment in the module specifying exactly what version of the upstream code this is based on
It all started a very long time ago as a fork of the Sphinx autodoc. Many changes have been made, some upstream changes were merged. It's not meaningful to say that it is a fork of a single upstream commit, it's much messier than that.
comment:10 in reply to: ↑ 8 Changed 4 years ago by
Replying to embray:
compare to the original to see what the differences are.
When I created this ticket, Sage was using Sphinx 1.5.3 so I guess it's best to compare against that: https://github.com/sphinxdoc/sphinx/blob/v1.5.3/sphinx/ext/autodoc.py
comment:11 Changed 4 years ago by
 Reviewers set to Erik Bray
What I mean is to just add such a link in the comments in the source, sort of like I did in the comment near the top of https://git.sagemath.org/sage.git/tree/src/sage/cpython/_py2_random.py?id=3ee11e056d80c4a9163e5a8d1a8c92e12c34f25b
You can set positive review once that's done so we can move forward on this.
comment:12 Changed 4 years ago by
 Commit changed from 130be6d92070befcf662862c1900156656b5f1ec to 14b02b01c572840e1c7958a80a397af840f7b1ed
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
14b02b0  Move upstream changes to autodoc into Sage

comment:13 Changed 4 years ago by
 Status changed from needs_review to positive_review
comment:14 Changed 4 years ago by
 Branch changed from u/jdemeyer/ticket/22604 to 14b02b01c572840e1c7958a80a397af840f7b1ed
 Resolution set to fixed
 Status changed from positive_review to closed
This isn't perfect but it's a step in the right direction.
New commits:
Upgrade to Sphinx 1.5.3
Docbuild fixes for Sphinx 1.5.x
Allow running Sphinx without SSL
Upgrade docutils
Move upstream changes to autodoc into Sage