Opened 11 years ago

Closed 10 years ago

#12229 closed enhancement (fixed)

Add section in the developers manual about sagenb development

Reported by: Jeroen Demeyer Owned by: Minh Van Nguyen
Priority: blocker Milestone: sage-5.2
Component: documentation Keywords:
Cc: Jason Grout, Karl-Dieter Crisman Merged in: sage-5.2.beta0
Authors: Jason Grout, Karl-Dieter Crisman Reviewers: William Stein, Jeroen Demeyer, Punarbasu Purkayastha, Karl-Dieter Crisman, Keshav Kini
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: to be merged with #11080 Stopgaps:

Status badges

Description (last modified by Karl-Dieter Crisman)

Since some Sage tickets will require patches to sagenb, there should be a section in the Sage developers manual about how to contribute to the flask-sagenb project. We really want every Sage developer to be able to contribute to sagenb. It's okay if this requires some work (like installing git), but this should be documented well.

Context: http://groups.google.com/group/sage-notebook/t/3fa33efda8db5c94

Apply: trac_12229-sagenb-developer-doc.3.patch and trac-12229-manifest.patch and trac_12229-reviewer.patch.

Attachments (3)

trac-12229-manifest.patch (1.0 KB) - added by Jeroen Demeyer 11 years ago.
trac_12229-sagenb-developer-doc.3.patch (96.6 KB) - added by Punarbasu Purkayastha 11 years ago.
Updated patch with fixed git links
trac_12229-reviewer.patch (6.0 KB) - added by Karl-Dieter Crisman 10 years ago.

Download all attachments as: .zip

Change History (59)

comment:1 Changed 11 years ago by Jeroen Demeyer

Description: modified (diff)

comment:2 Changed 11 years ago by Karl-Dieter Crisman

Cc: Karl-Dieter Crisman added

comment:3 Changed 11 years ago by Jason Grout

This is a preliminary patch, based on the gitwash project for generating documentation for github projects: https://github.com/matthew-brett/gitwash

Any general improvements to this documentation should be contributed back to the gitwash project. Also, we should probably add the paragraph or two in the Development section in the sagenb README.rst about converting between hg and git repositories: https://github.com/sagemath/sagenb/blob/master/README.rst

Ideally, sagenb would be documented totally separately in its source tree, and that documentation would somehow be available to the users.

comment:4 Changed 11 years ago by Jason Grout

Status: newneeds_review

Okay, I added a little bit about switching to git, almost straight from the sagenb readme. I think it's ready for review, though we should eventually move this to sagenb.

comment:5 Changed 11 years ago by William Stein

Status: needs_reviewpositive_review

After hours of work and nitpicking... positive review!

comment:6 Changed 11 years ago by Jeroen Demeyer

Authors: Jason Grout
Reviewers: William Stein

comment:7 in reply to:  4 ; Changed 11 years ago by Jeroen Demeyer

Replying to jason:

We should eventually move this to sagenb.

Hmm, I'm not sure about this. The documentation should be obviously findable for non-sagenb developers.

comment:8 in reply to:  7 Changed 11 years ago by Jason Grout

Replying to jdemeyer:

Replying to jason:

We should eventually move this to sagenb.

Hmm, I'm not sure about this. The documentation should be obviously findable for non-sagenb developers.

Yes, that is the reason why we are not doing that right now---something needs to be figured out to make that doc easily accessible. This ticket just adds the docs to the sage developer's guide for now.

comment:9 Changed 11 years ago by Jeroen Demeyer

Status: positive_reviewneeds_work

Currently, the commit message of this patch has some very long lines. Could you wrap these long lines? Make sure the first line (shown by hg log) makes sense by itself.

comment:10 Changed 11 years ago by Ian Stokes-Rees

Description: modified (diff)
Status: needs_workpositive_review

comment:11 Changed 11 years ago by Jeroen Demeyer

Status: positive_reviewneeds_work

make doc-html-jsmath fails:

sphinx-build -b html -d /home/buildbot/build/sage/eno-1/eno_full/build/sage-5.0.beta0/devel/sage/doc/output/doctrees/en/developer   -A hide_pdf_links=1 /home/buildbot/build/sage/eno-1/eno_full/build/sage-5.0.beta0/devel/sage/doc/en/developer /home/buildbot/build/sage/eno-1/eno_full/build/sage-5.0.beta0/devel/sage/doc/output/html/en/developer
Running Sphinx v1.1.2
loading pickled environment... not yet created
building [html]: targets for 27 source files that are out of date
updating environment: 27 added, 0 changed, 0 removed
reading sources... [  3%] coding_in_cython
reading sources... [  7%] coding_in_other
reading sources... [ 11%] coding_in_python
reading sources... [ 14%] conventions
reading sources... [ 18%] disseminating_code
reading sources... [ 22%] doctesting
reading sources... [ 25%] inclusion
reading sources... [ 29%] index
reading sources... [ 33%] patching_spkgs
reading sources... [ 37%] producing_patches
reading sources... [ 40%] producing_spkgs
reading sources... [ 44%] sage_manuals
reading sources... [ 48%] sagenb/configure_git
reading sources... [ 51%] sagenb/development_workflow
reading sources... [ 55%] sagenb/following_latest
reading sources... [ 59%] sagenb/forking_hell
reading sources... [ 62%] sagenb/git_development
reading sources... [ 66%] sagenb/git_install
reading sources... [ 70%] sagenb/git_intro
reading sources... [ 74%] sagenb/git_resources
reading sources... [ 77%] sagenb/index
reading sources... [ 81%] sagenb/maintainer_workflow
reading sources... [ 85%] sagenb/patching
reading sources... [ 88%] sagenb/set_up_fork
reading sources... [ 92%] trac
reading sources... [ 96%] walk_through
reading sources... [100%] writing_code

/home/buildbot/build/sage/eno-1/eno_full/build/sage-5.0.beta0/devel/sage/doc/en/developer/sagenb/configure_git.rst:158: SEVERE: Problems with "include" directive path:
IOError: [Errno 2] No such file or directory: '../../../../sage/doc/en/developer/sagenb/links.inc'.
/home/buildbot/build/sage/eno-1/eno_full/build/sage-5.0.beta0/devel/sage/doc/en/developer/sagenb/configure_git.rst:58: ERROR: Unknown target name: "git".
/home/buildbot/build/sage/eno-1/eno_full/build/sage-5.0.beta0/devel/sage/doc/en/developer/sagenb/development_workflow.rst:415: SEVERE: Problems with "include" directive path:
IOError: [Errno 2] No such file or directory: '../../../../sage/doc/en/developer/sagenb/links.inc'.
/home/buildbot/build/sage/eno-1/eno_full/build/sage-5.0.beta0/devel/sage/doc/en/developer/sagenb/development_workflow.rst:20: ERROR: Undefined substitution referenced: "emdash".
/home/buildbot/build/sage/eno-1/eno_full/build/sage-5.0.beta0/devel/sage/doc/en/developer/sagenb/development_workflow.rst:137: ERROR: Undefined substitution referenced: "emdash".
[...many more...]

comment:12 Changed 11 years ago by Jason Grout

I'm trying to reproduce this, but everything works for me. I deleted the output for the developer's guide and ran make-doc-jsmath, and there were no complaints.

Is the error reproducible on eno? That's on skynet? I might be able to log in and try to reproduce it there.

comment:13 in reply to:  12 Changed 11 years ago by Jeroen Demeyer

Replying to jason:

I'm trying to reproduce this, but everything works for me. I deleted the output for the developer's guide and ran make-doc-jsmath, and there were no complaints.

Is the error reproducible on eno? That's on skynet? I might be able to log in and try to reproduce it there.

It's reproducible on every machine on the Buildbot. But I just think of something: are you adding files which are not in devel/sage/MANIFEST.in?

comment:14 Changed 11 years ago by Jason Grout

I didn't touch the MANIFEST.in. That probably explains the problem. I'll post a patch.

comment:15 Changed 11 years ago by Jason Grout

Status: needs_workneeds_review

The attached patch should fix the MANIFEST.in issues.

comment:16 Changed 11 years ago by Jeroen Demeyer

Description: modified (diff)

comment:17 Changed 11 years ago by Jeroen Demeyer

Since it's just the MANIFEST.in thing, I'll have a look at it. I forgot this still needed review...

comment:18 Changed 11 years ago by Punarbasu Purkayastha

The links to kernel.org in doc/en/developer/sagenb/git_links.inc no longer work. Is that just a fallout of the kernel.org downtime from a couple of months ago, or has the location of the docs changed? From a quick Google search, I don't see any other "official" place which can be referred.

comment:19 in reply to:  18 Changed 11 years ago by Karl-Dieter Crisman

Reviewers: William SteinWilliam Stein, Jeroen Demeyer, Punarbasu Purkayastha

The links to kernel.org in doc/en/developer/sagenb/git_links.inc no longer work. Is that just a fallout of the kernel.org downtime from a couple of months ago, or has the location of the docs changed? From a quick Google search, I don't see any other "official" place which can be referred.

It looks like most of the doc has moved, e.g. the user manual and the tutorial. Since everything looks the same, just with the http://shacon.github.com/git/ prefix, presumably that's all that has to be changed. In fact, it looks like some kind of sed on the patch replacing http://www.kernel.org/pub/software/scm/git/docs with http://schacon.github.com/git would suffice.

So for positive review one needs

  • to fix the links (which wouldn't even need a separate export, it could just be done on the patch file)
  • to check the MANIFEST.in patch solves the other problem, which I won't touch with a ten-foot pole.

comment:20 Changed 11 years ago by Jeroen Demeyer

Positive review to trac-12229-manifest.patch (I posted a new version which is simply a rebase).

comment:21 Changed 11 years ago by Jason Grout

The index.rst also needs to be updated to reflect the move to git and github.

Changed 11 years ago by Jeroen Demeyer

Attachment: trac-12229-manifest.patch added

comment:22 Changed 11 years ago by Jeroen Demeyer

Milestone: sage-5.0sage-5.1

comment:23 Changed 11 years ago by Jeroen Demeyer

Description: modified (diff)

comment:24 Changed 11 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work
Work issues: Google code

I think the documentation should mention http://code.google.com/p/sagenb/ and what its purpose is, something equivalent to http://boxen.math.washington.edu/home/release/sage-5.0.notebook/doc/en/developer/trac.html. On that page about Trac, add a comment that this should not be used for Notebook-specific things (unfortunately, it's not always clear whether a bug is notebook-specific).

comment:25 Changed 11 years ago by Jason Grout

code.google.com serves no purpose any more. All development has moved to github. There may be tickets at code.google.com that still need to be moved over, thought.

comment:26 in reply to:  25 ; Changed 11 years ago by Jeroen Demeyer

Status: needs_workneeds_review
Work issues: Google code

Replying to jason:

code.google.com serves no purpose any more. All development has moved to github. There may be tickets at code.google.com that still need to be moved over, thought.

Then I guess http://nb.sagemath.org/ should be updated too.

comment:27 in reply to:  26 ; Changed 11 years ago by Punarbasu Purkayastha

Replying to jdemeyer:

Then I guess http://nb.sagemath.org/ should be updated too.

And the google code page should inform the reader that they should look at github.

comment:28 in reply to:  27 Changed 11 years ago by Jason Grout

Replying to ppurka:

And the google code page should inform the reader that they should look at github.

Done.

comment:29 Changed 11 years ago by Keshav Kini

It would be nice if we could keep this documentation in sagenb itself and somehow link it into the Sage doc building process. But I guess that can be fixed later.

BTW, for reasons I stated here I don't think we should be teaching people how to rebase...

Changed 11 years ago by Punarbasu Purkayastha

Updated patch with fixed git links

comment:30 Changed 11 years ago by Punarbasu Purkayastha

Description: modified (diff)

The git documentation url hasn't changed in several months, so I updated the patch with the fixed documentation. The diff file shows the changes to the patch.

Unfortunately, I don't have permission to overwrite the earlier patch. So, I uploaded a differently named one.

comment:31 Changed 11 years ago by Punarbasu Purkayastha

A question: where does the documentation get built? And how can I view it? I don't see it from the help link in the notebook.

comment:32 in reply to:  31 Changed 11 years ago by Punarbasu Purkayastha

Replying to ppurka:

A question: where does the documentation get built? And how can I view it? I don't see it from the help link in the notebook.

Sorry, I needed to build the documentation. Now it appears.

comment:33 Changed 11 years ago by Karl-Dieter Crisman

It would be nice to know what is remaining to review here.

comment:34 Changed 11 years ago by Keshav Kini

patchbot: apply trac_12229-sagenb-developer-doc.3.patch trac-12229-manifest.patch

comment:35 Changed 11 years ago by Karl-Dieter Crisman

Description: modified (diff)

comment:36 Changed 11 years ago by Karl-Dieter Crisman

Reviewers: William Stein, Jeroen Demeyer, Punarbasu PurkayasthaWilliam Stein, Jeroen Demeyer, Punarbasu Purkayastha, Karl-Dieter Crisman
Status: needs_reviewneeds_work

Okay, I have a few questions. Mostly this is because of the annoying autogenerated thing this is from.

  • The file this_project.inc has
    .. Sage Notebook
    .. _`Sage Notebook`: http://sagemath.org
    .. _`Sage Notebook github`: http://github.com/sagemath/sagenb
    
    .. _`Sage Notebook mailing list`: http://groups.google.com/group/sage-notebook
    
    but I doubt we want the Sage notebook site to be sagemath.org, as there are a number of links to this. What is the best "generic" Sage notebook URL?
  • I don't understand why there are several references to sending files to the Sage notebook devel list. That is really not at all what we want it to do! So what do we want it to do? Surely not to upload to Trac ;-)
  • "Hardcore forking action" is semi-offensive and unfortunately part of the old crude hacking humor thing. We should delete it.
  • There is a lot of reference to just sagenb, but in reality one might want it to be inside the devel directory (well, or whatever happens in the "new" Sage tree someday).

My recommendation: make it really really obvious that this is absolutely generic documentation, and fix the things above. This should all be doable in index.rst and a few other places. I can do some of this, but am not sure what the answers to the first two questions are.

Also, although this should still be a blocker for whatever release the new Sage notebook goes in, I don't think that the new notebook should depend on it; we want people testing it as soon as possible. By the way, once the new notebook goes in, do we still need the removal and recreation of the symlink? I guess so...

comment:37 Changed 11 years ago by Jeroen Demeyer

Milestone: sage-5.1sage-5.2

comment:38 in reply to:  36 ; Changed 11 years ago by Keshav Kini

Replying to kcrisman:

Okay, I have a few questions. Mostly this is because of the annoying autogenerated thing this is from.

I agree about it being annoying - why are we using an autogenerated thing, anyway? Our mercurial docs aren't autogenerated.

  • The file this_project.inc has
    .. Sage Notebook
    .. _`Sage Notebook`: http://sagemath.org
    .. _`Sage Notebook github`: http://github.com/sagemath/sagenb
    
    .. _`Sage Notebook mailing list`: http://groups.google.com/group/sage-notebook
    
    but I doubt we want the Sage notebook site to be sagemath.org, as there are a number of links to this. What is the best "generic" Sage notebook URL?

Well, ideally it is http://nb.sagemath.org/ , but that page is extremely outdated right now. Someone should update it.

  • I don't understand why there are several references to sending files to the Sage notebook devel list. That is really not at all what we want it to do! So what do we want it to do? Surely not to upload to Trac ;-)
  • "Hardcore forking action" is semi-offensive and unfortunately part of the old crude hacking humor thing. We should delete it.

Those actual words appear on github when you fork a repository, by the way.

  • There is a lot of reference to just sagenb, but in reality one might want it to be inside the devel directory (well, or whatever happens in the "new" Sage tree someday).

Once the whole Sage tree switches to git (#13015), this autogenerated stuff will be even less adequate. At least now it seems that most sagenb contributors are fairly experienced at contributing to open source projects. That is not true of sage library contributors.

My recommendation: make it really really obvious that this is absolutely generic documentation, and fix the things above. This should all be doable in index.rst and a few other places. I can do some of this, but am not sure what the answers to the first two questions are.

Also, although this should still be a blocker for whatever release the new Sage notebook goes in, I don't think that the new notebook should depend on it; we want people testing it as soon as possible. By the way, once the new notebook goes in, do we still need the removal and recreation of the symlink? I guess so...

You mean the symlink $SAGE_ROOT/devel/sagenb? Of course not :)

comment:39 in reply to:  38 ; Changed 11 years ago by Karl-Dieter Crisman

Okay, I have a few questions. Mostly this is because of the annoying autogenerated thing this is from.

I agree about it being annoying - why are we using an autogenerated thing, anyway? Our mercurial docs aren't autogenerated.

Because silly people like me complained that the sagenb should have documentation for how to develop included when upgraded, and this was convenient. Really, the autogen is not a problem, it's that it hasn't been made more clear what is boilerplate.

  • The file this_project.inc has

.. _Sage Notebook: http://sagemath.org

Well, ideally it is http://nb.sagemath.org/ , but that page is extremely outdated right now. Someone should update it.

Right, but who can do that? I don't know who has admin rights to that other than William, who isn't really involved in this upgrade that much.

  • I don't understand why there are several references to sending files to the Sage notebook devel list. That is really not at all what we want it to do! So what do we want it to do? Surely not to upload to Trac ;-)

Answering my own question - presumably to open an issue on github and post code there?

  • "Hardcore forking action" is semi-offensive and unfortunately part of the old crude hacking humor thing. We should delete it.

Those actual words appear on github when you fork a repository, by the way.

Totally unprofessional.

  • There is a lot of reference to just sagenb, but in reality one might want it to be inside the devel directory (well, or whatever happens in the "new" Sage tree someday).

Once the whole Sage tree switches to git (#13015), this autogenerated stuff will be even less adequate. At least now it seems that most sagenb contributors are fairly experienced at contributing to open source projects. That is not true of sage library contributors.

I would distinguish between "contributing to open source projects" and "using these specific tools in such contribution. Anyway, for now I think this is less problematic.

My recommendation: make it really really obvious that this is absolutely generic documentation, and fix the things above. This should all be doable in index.rst and a few other places. I can do some of this, but am not sure what the answers to the first two questions are.

Also, although this should still be a blocker for whatever release the new Sage notebook goes in, I don't think that the new notebook should depend on it; we want people testing it as soon as possible. By the way, once the new notebook goes in, do we still need the removal and recreation of the symlink? I guess so...

You mean the symlink $SAGE_ROOT/devel/sagenb? Of course not :)

So yet another thing that should be changed.

Keshav, do you think you can take a stab at making a new patch with these changes? Or ppurka if he is participating in the notebook days remotely. I figure that this would be pretty compelling for a good use of time at a notebook days :) but let me know if not; probably over the next couple weeks I could do a very bad job at fixing a few of them.

Maybe you can bug someone about the nb.sagemath.org stuff this week too. That would be really great.

comment:40 in reply to:  39 Changed 11 years ago by Karl-Dieter Crisman

Maybe you can bug someone about the nb.sagemath.org stuff this week too. That would be really great.

Update: sounds like we can just ask Harald to do something. I've done that.

We definitely do need some ideas on what the right place to send people is for developing. The current page here is not right at all, but I don't know whether pointing people to this documentation is best or not. However, I don't see much on the github site that is accessible, and we don't want to point them to #11080 once the new notebook is in!

Last edited 10 years ago by Karl-Dieter Crisman (previous) (diff)

comment:41 Changed 10 years ago by Jason Grout

+1 to KDC's comments about changing the Hardcore thing...

comment:42 Changed 10 years ago by Karl-Dieter Crisman

I haven't heard back from Harald.

This is clearly not an ideal setup overall, but as long as we have a very clear first page saying that everything is generic, and giving all relevant links, it's better than not having documentation at all. I don't know who would have time, interest, and expertise to do something more, though that would clearly be better.

comment:43 in reply to:  42 Changed 10 years ago by Karl-Dieter Crisman

I haven't heard back from Harald.

Okay, he has graciously updated the page a lot. If we change

.. _`Sage Notebook`: http://sagemath.org

to

.. _`Sage Notebook`: http://nb.sagemath.org

in this_project.inc that will solve many of our problems.

This is clearly not an ideal setup overall, but as long as we have a very clear first page saying that everything is generic, and giving all relevant links, it's better than not having documentation at all. I don't know who would have time, interest, and expertise to do something more, though that would clearly be better.

I'm just going to change the things that need to be done myself. We need to get this stuff in. Patch hopefully coming up.

Changed 10 years ago by Karl-Dieter Crisman

Attachment: trac_12229-reviewer.patch added

comment:44 Changed 10 years ago by Karl-Dieter Crisman

Authors: Jason GroutJason Grout, Karl-Dieter Crisman
Description: modified (diff)
Status: needs_workneeds_review

Patchbot: apply trac_12229-sagenb-developer-doc.3.patch and trac-12229-manifest.patch and trac_12229-reviewer.patch

All others: I think this is now ready for review. It was very tempting to completely rewrite this but having to get something done today won out. All that requires review is the reviewer patch, including that documentation builds correctly.

comment:45 Changed 10 years ago by Keshav Kini

Reviewers: William Stein, Jeroen Demeyer, Punarbasu Purkayastha, Karl-Dieter CrismanWilliam Stein, Jeroen Demeyer, Punarbasu Purkayastha, Karl-Dieter Crisman, Keshav Kini
Status: needs_reviewpositive_review

This is good enough for now. We already have stuff lined up for the successor to #11080, so let's at least get #11080 in ASAP :)

comment:46 Changed 10 years ago by Keshav Kini

To quote my recent comment on #11080:

John Palmieri, who is sitting next to me at sage days 41, suggests that we just not package a repository with sagenb at all. What do you think? After all, sagenb is considered an upstream package at this point, and we don't package repositories for those. Plus it's using a revision control system that isn't included with sage anyway (yet). Also, due to various decisions by people over the lifetime of the repository, the repository is really large - 14 MB at maximum compression, at the current master. So removing it would slim down the sage source and binary distributions by a non-negligible amount. The docs added at #12229 already give instructions on how to clone the repository from github anyway. So why do we need to ship a repo?

comment:47 Changed 10 years ago by Jason Grout

I guess the question is if we want it to be easy for sage developers to jump into sagenb development. I say yes, but I'm a bit biased.

comment:48 Changed 10 years ago by Keshav Kini

Well, we always want things to be easy, so that question has a clear answer, but there are other questions too, such as whether we want to double the size of the sagenb SPKG, or whether we want to ship more binary data, etc. etc. In any case it's not really much easier to develop for sagenb if we ship a repository, because you still need to install git on your system, or install the git SPKG. After that, cloning is a single command, and how to run that command is even explained in the patches on this ticket.

comment:49 Changed 10 years ago by Jason Grout

But then you need to switch the sagenb symbolic link and do another setup.py develop.

Maybe an optional sagenb-dev spkg is the solution?

comment:50 Changed 10 years ago by Keshav Kini

IMO switching a symbolic link and doing setup.py develop is not an unreasonable thing to expect a potential sagenb developer to do. At the same time as making things easy for sage developers who want to hack on sagenb, I also want to make things easy for whoever is making sagenb SPKGs (currently me, so that's a kind of selfish desire :) ).

comment:51 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.2.beta0
Resolution: fixed
Status: positive_reviewclosed

comment:52 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.2.beta0
Milestone: sage-5.2sage-pending
Resolution: fixed
Status: closednew

Unmerging this from sage-5.2 due to the serious security issue at #13270.

comment:53 Changed 10 years ago by Jeroen Demeyer

Status: newneeds_review

comment:54 Changed 10 years ago by Jeroen Demeyer

Status: needs_reviewpositive_review

comment:55 Changed 10 years ago by Jeroen Demeyer

Dependencies: to be merged with #11080

comment:56 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.2.beta0
Milestone: sage-pendingsage-5.2
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.