Use "git trac" in the developer guide

vbraun commented 10 years ago

Component: documentation

Author: Volker Braun, Karl-Dieter Crisman

Branch/Commit: 9b04b3f

Reviewer: Ralf Stephan, Volker Braun, Karl-Dieter Crisman

Issue created by migration from https://trac.sagemath.org/ticket/16030

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 10 years ago

comment:1

(curious)

vbraun commented 10 years ago

Branch: u/vbraun/use__git_trac__in_the_developer_guide

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Commit: 49ee6a5

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

49ee6a5 add "git trac" to the developer manual

vbraun commented 10 years ago

Author: Volker Braun

rwst commented 10 years ago

Reviewer: Ralf Stephan

rwst commented 10 years ago

comment:6

HTML looks good. Typos found:

ultimaltely wasted
commandline
checkout a ticktet
explictly add

Assuming the necessity of it, and that the given commands do what you say, please set positive after the typos are gone.

kcrisman commented 10 years ago

comment:7

I agree with rws that it's not 100% clear why this is necessary, and I won't be looking for typos or syntax errors (though it mostly looks fine assuming git trac does the same thing as sage -dev in some sense), but did find one things that should be fixed even if one agrees with "the author".

This section gives an example how to review using the ``sage``

One of the times that shows up you actually have used git trac - toward the end of the diff.

Long comment addressed (presumably) to Volker, though of course anyone else who has been writing things like the sage -dev or git trac stuff is more than welcome to join in:

The overly dismissive tone about the dev scripts is unnecessary, and to be honest seems a little insulting to the people who worked hard on creating those - which doesn't include me, and t think might even include said author of the guide, which confuses me?

In any case, relegating the dev script part to something that isn't really even mentioned seems... odd. I think that "the author" has an overly optimistic view of how easy it is to overcome all the hurdles associated with development. And yes, Virginia, some open source projects don't collaborate using git. It would be helpful to have this discussion on sage-devel about this, rather than a somewhat combative tone in what is supposed to be an educational document providing the easiest path to doing things like reviewing a patch. A new reader will be really confused by this - imagine someone who is starting to use a terminal for the first time encountering the vim/emacs debate; it would just turn them off.

Honestly, not having to do things like install git (which is, after all, in Sage) or all the other stuff involved really helps a true newcomer. I mean, look how much one has to do before getting to reviewing. (That is a problem with the current layout of the devel guide as well, but is made more egregious by this change). I am going to be teaching Sage and its development to mathematicians (not students) soon, many of whom are "completely new to programming" (quote from the director of the program). I totally understand that one wants to use git for serious developing, just like before using hg and its queues directly were the right thing to do. But I really hesitate to try to prepare a syllabus for this without having the "first step mentioned" be the very easiest, even if some non-trivial percentage abandon it later for directly interacting with git.

I'm willing to help work on a different layout for the devel guide that addresses your concerns about not having enough git resources and mine about making sure the most basic interface is prominent (and correct, which as you note the documentation isn't always, or complete). It will take some time, because I have very limited Sage time right now, but I hope you'll be willing to work with me on this.

vbraun commented 10 years ago

comment:9

I've worked on the dev scripts a lot myself, so if anything I'm dismissing my work. Also, we had a quick informal survey at the last sage days and most developers don't use the dev scripts. And if so mostly for the feature to import old patches. So realistically the dev scripts won't be maintained or improved. And there are some really annoying bugs in them that imho already make the initial setup already more painful than just installing git (which is nowadays installed by default in most distros and part of XCode).

Of course if you want to further work on the dev scripts then thats fine with me. And/or post to sage-devel to get a discussion going about our roadmap...

rwst commented 10 years ago

comment:10

Replying to @kcrisman:

I agree with rws that it's not 100% clear why this is necessary

This is an assumption. "Assuming the necessity ..." can equally mean that *I am assuming it. I do but I have much less overview than you both.

kcrisman commented 10 years ago

comment:11

I've worked on the dev scripts a lot myself, so if anything I'm dismissing my work.

Fair enough, I suspected this.

Also, we had a quick informal survey at the last sage days and most developers don't use the dev scripts. And if so mostly for the feature to import old patches. So realistically the dev scripts won't be maintained or improved. And there are some really annoying bugs in them that imho already make the initial setup already more painful than just installing git (which is nowadays installed by default in most distros and part of XCode).

Of course if you want to further work on the dev scripts then thats fine with me. And/or post to sage-devel to get a discussion going about our roadmap...

It's not so much about further work, as making sure there is SOME low bar to entry. I'm not worried about current developers; hardly any of them were using hg_sage toward the end either, but there are always people who need that level of entry. "Every user a developer" is more what I'm thinking, even though of course that is never true in practice. If "git trac" ends up being very easy to use, that's fine; the current instructions on this ticket looked rather overwhelming, but perhaps reorganization of that section would change that impression?

With that in mind, before I open a discussion, I guess I'm curious what your roadmap actually is.

vbraun commented 10 years ago

comment:12

I agree of course that the bar of entry should be as low as possible. One thing that I could do is start with just "readonly" access where you get the sources and make changes. And then have a separate section talk about uploading those changes. What do you think about that?

About my roadmap, I've been developing the git trac script over the last 6 months and they form the core of my release management workflow. I've implemented pretty much everything that I think should go in there, so now its time to start steering people towards using them. This is part of the reason for being very frank about sage -dev, I don't want to dis anybody but I want to steer any newcomer in the right (or what I think is right ;-) direction.

kcrisman commented 10 years ago

comment:13

I'll think about that, thanks - as long as this isn't a ticket that has to get merged immediately, which it seems not, I appreciate it.

As to sage -dev command versus git trac command, if they really are nearly identical in most ways, I don't see a problem; the issue would be if ... hold on, could one simply make sage -dev command equal sage -git trac command in some obvious 1-1 way? (I mean that sage -dev command would literally call sage -git trac command behind the scenes - in which case one doesn't need to separately download git but still gets all the benefit; I noticed git_trac in the latest sources, though perhaps that is not a built pkg?) But I don't know if they have different functionalities in some places. After all, if git is coming with Sage anyway, why download it? (For the newish developer, I mean.)

vbraun commented 10 years ago

comment:14

I think it would be terrible to present users with two competing models. We need to be clear which one we prefer, anything else is just confusing to newcomers. Note that I'm fine with documenting different approaches. So -1 to a rosetta stone, but +1 for a one-page "git cheat sheet" that you can print out and put next to your computer.

Also, I'm against using the git install inside of Sage. That'll just cause tears later: If make distclean && make didn't work you now you can't switch branches, yay. Especially with even Xcode including git now there really is no point.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

49404ac splitting the walkthrough in a local and a remote part

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from 49ee6a5 to 49404ac

vbraun commented 10 years ago

comment:16

doc output here: http://boxen.math.washington.edu/home/vbraun/doc/developer/

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from 49404ac to 86a4216

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

86a4216 also mention the cheat sheet

kcrisman commented 10 years ago

comment:18

Okay, agreed on the Rosetta stone, that makes sense.

I hope it's okay if I don't immediately respond on this - I have been so incredibly swamped, and shouldn't even really be doing anything Sage-related, but this and the jsmol are pretty important to me - I think I can think of a good reorganization. Mainly, this is so git-centric early on that someone who just wants to see anything at all about what it even would mean to help develop is lost. But git trac looks promising.

vbraun commented 10 years ago

comment:19

You are of course welcome to improved the docs later, but I think the current state is enough of an improvement over what we have that it should go in asap.

On a practical level, we have to start with "here is how to make a branch". If you don't make a branch FIRST then that means you edit the master and that will cause a lot of pain later on.

kcrisman commented 10 years ago

comment:20

On a practical level, we have to start with "here is how to make a branch". If you don't make a branch FIRST then that means you edit the master and that will cause a lot of pain later on.

Right, but if you don't know why you are making a branch it is even more confusing. For now, it's that part of the organization I'm referring to - it assumes quite a bit of what the reader wants to do. What is nice about the current format is that it says stuff about what to do for the most common use cases first, leaving things like setting up git or even acquiring a Trac account to later. What should a brand-new developer want to do? Review, probably, or maybe make a new ticket. This piece is pretty skeletal, and sage -b/make aren't even mentioned. This is why I don't think your current revisions are (yet) an improvement for that new person.

BUT I think it won't be terribly hard to make them so, which I why I want to work with you on this. Next week (especially after a certain important day involving lots of paperwork for US folks) at the latest I can devote time to this, and I would think that is not too long, given the many betas which have already come through without it.

Perhaps along these lines, here is something either wrong or confusing.

[user@localhost sage]$ git checkout master
[user@localhost sage]$ git branch last_twin_prime
[user@localhost]$ git branch
  master
* last_twin_prime

But it says "Also note that git branch creates a new branch, but does not switch to it. For this, you have to use git checkout:" - yet the star is on last_twin_prime.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

4b8db3d add missing checkout command

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from 86a4216 to 4b8db3d

vbraun commented 10 years ago

comment:22

Ok, having instructions to just checkout a ticket and build Sage with it would be useful if you a bugfix from a ticket. I would have put that into the installation manual, but I guess it could go into the dev manual as well. I don't think its useful to have review instructions at the top though, if you have never authored a ticket then you shouldn't be reviewing stuff.

I added the missing checkout command.

kcrisman commented 10 years ago

comment:23

Ok, having instructions to just checkout a ticket and build Sage with it would be useful if you a bugfix from a ticket. I would have put that into the installation manual, but I guess it could go into the dev manual as well. I don't think its useful to have review instructions at the top though, if you have never authored a ticket then you shouldn't be reviewing stuff.

Really? That is exactly the opposite of my strategy. My experience is that a lot of people are very reluctant to add code (see all the very tentative emails we get on this "is it good enough for Sage? where do I submit it?") but reviewing (at least a partial review) is a great way to see how things work. Probably a first-timer on review should have a helping hand. But it's much easier to verify that especially fairly routine bugs are fixed and doctested than to add things, lots of times. There are fewer steps for reviewing, too. Of course there are many tickets for which you don't want this (and people are usually extremely hesitant to review tickets they feel unqualified for), but I've found it's a sneaky way to get people to do more developing.

I added the missing checkout command.

Thanks.

novoselt commented 10 years ago

comment:24

I've just installed these scripts, so while novice impressions are fresh:

I have cloned the original repository from trac following "Tentative conventions" wiki, so my "origin" in git already points to trac and seems a bit silly to add it as "trac" as well. In general, it would be nice to have some clear statement in the "setting up" of why there is a github version as well and when one should use it. Maybe there is already something somewhere, but I don't recall seeing it. I do recall someone (Volker?) saying that it saves bandwidth.
There are way too many versions of installing "git trac" ;-) And "python setup.py install" has to be run as sudo, would be nice to add it to the documentation. I guess this is why there is also an option of just adding it to PATH.
"Configuration" gives two commands for setting remote trac as read-only or read-write, and only one of the two has to be used, which is not immediately clear. And who has to use read-only anyway?
Is it possible to have "parameter TAB completion"? I.e. when I type "git trac " I'd like to see these "create", "push" etc. as it happens with regular git commands, but instead I get local file listing.
Enabling "git help trac" would be nice as well!
Perhaps password in the example can be entered in single quotes to avoid problems with special characters, i.e.

$ git trac config --user=Myself --pass='s3kr1t'

And adding there an appropriate chmod would be nice since by default .git/config is 0644

$ chmod 0600 .git/config

novoselt commented 10 years ago

comment:25

One more random idea: can "git trac create" open the browser after it is done for filling in all the fields? The link is printed in the terminal already, so it is easy to do manually, but perhaps it will help keeping book-keeping organized.

rwst commented 10 years ago

comment:26

@novoselt: Your suggestions appear to be better placed in https://github.com/sagemath/git-trac-command/issues while this ticket is about the patch to the Sage dev docs. Can you please open your issues there?

rwst commented 10 years ago

comment:27

Having reviewed this already (and the added patch), as well as having used git-trac for some time, I'm confident in giving this positive.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

d25e691 reviewer suggestions

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from 4b8db3d to d25e691

vbraun commented 10 years ago

comment:29

Andrey: I recently added a "git trac browse" command that'll open the trac web page of the current ticket. I'm not sure if that should be done automatically after a ticket creation; Especially for simple changes I prefer to open the ticket, work on the code, then fill out the details on the web page. Though one can certainly argue that the browser should be opened first. But in the end I think less is more.

Tab completion is a shell feature. I've addressed the other comments in my last commit.

Karl-Dieter: I'm definitely interested in making it more accessible to newcomers, but I suggest we do it on a followup ticket.

kcrisman commented 10 years ago

comment:30

Karl-Dieter: I'm definitely interested in making it more accessible to newcomers, but I suggest we do it on a followup ticket.

Darn, I finally had removed enough from my plate to work on this today and tomorrow (I even opened my shell on the train, only to discover no wifi today, how annoying).

If you are already in rc mode on 6.2 maybe this should be in 6.3 anyway... several recent emails suggest at the very least sage -dev needs to be much better documented, as you have mentioned, so I see why you are thinking about this. I just hate that someone is directed first to install something else in order to even start thinking of developing Sage. :-(

kcrisman commented 10 years ago

comment:31

I'm getting

[developer] /Users/.../sage/src/doc/en/developer/workflows.rst:31: WARNING: undefined label: section-git-branch (if the link has no caption the label must precede a section header)

kcrisman commented 10 years ago

comment:32

And indeed, in src/doc/en/developer/manual_git.rst there is no section-git-branch nor section-git-commit. I'm not sure why this compiled properly for you; I did use ./sage -docbuild developer html because make doc broke for me somewhere else with the branch switch.

vbraun commented 10 years ago

comment:33

Thanks, sphinx caching got me... I changed the links anchors but not workflows.rst so that file was never sphinxed.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

36e7fcb fix links

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from d25e691 to 36e7fcb

vbraun commented 10 years ago

comment:35

I should probably add a quick section in the beginning about the sage -b vs make thing, will do that right now.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from 36e7fcb to e1137d9

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

e1137d9 add section on how to rebuild sage

vbraun commented 10 years ago

comment:37

Done

kcrisman commented 10 years ago

comment:38

I should probably add a quick section in the beginning about the sage -b vs make thing, will do that right now.

Yes, that is a very good catch - I'm still confused about it myself!

I think that through the course of today I can make some minimal changes with respect to organization that would make it just a little clearer without putting sage -dev in a position of prominence. For instance, it's confusing that git-trac.html is in the initial area (should it be "collaborative", not "collaborate", development?). Is that reasonable? At this point you have some other input so I'm not trying to hold it up, but would prefer to make sure a new person can read this in order, at least; other stuff can wait at this point.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from e1137d9 to e3259d0

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

e3259d0 use git trac print instead of get

vbraun commented 10 years ago

comment:40

Google search seems to be in favor of "collaborative development" by far.

For a complete newbie approach it might be nice to have a sage upgrade 12345 to just download the ticket branch and rebuild sage without any user intervention. We can do that in the followup.

kcrisman commented 10 years ago

comment:41

Google search seems to be in favor of "collaborative development" by far.

Okay, let me fix that and try to slightly rearrange this somewhat today. My main ideas would be (none of which would take very long once I have an uninterrupted stretch)

Make top of index.html a little more organized, showing coming attractions (for those reading it and not just searching in it)
Rename "Collaborate Development" to "Collaborative Development with Git Trac" and make it its own (sub)section
Probably making the "Writing Code for Sage" a main section, while putting all the git/trac related stuff into a separate main section
Putting the notebook developer stuff at the end again (and possibly warning that some of the info may be outdated)
Miscellaneous wording fixes.

For a complete newbie approach it might be nice to have a sage upgrade 12345 to just download the ticket branch and rebuild sage without any user intervention. We can do that in the followup.

Yes, that is true, but you are right it would definitely be outside the scope of this ticket - and I'm not sure I'd want "upgrade" as the word.

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 10 years ago

comment:42

Replying to @kcrisman:

I just hate that someone is directed first to install something else in order to even start thinking of developing Sage. :-(

And it's IMHO a bit weird to assume someone would start developing Sage without (ever?) having it installed...

Of course there are also binary dists, but at least the source code of the Sage library is always included, and I think most users will have taken a look at it before considering contributing, or will at least have seen it in tracebacks ;-) or by having typed foo??.

Still, there should probably be a special section or a whole chapter for "absolute beginners", starting e.g. with how to fix a typo in the documentation, i.e., where to find the relevant file(s), how to edit them, how to rebuild the docs (and also the Sage library, in case of docstrings), then introducing what revision control systems are all about, that Sage (now) uses git, and how Sage development is organized (trac -- not just a bugtracker, but used for all Sage development; "stable" releases vs. "development" releases, how reviewing happens, the relevant mailing lists...) -- and the latter mostly in prose, without terminal screen dumps, and without mentioning dozens of commands and options (whether with or without referencing other sections) in the first place.

(I don't mean this should be added on this ticket, and IMHO we should get what we have into Sage 6.2, since then presumably a lot more people will give feedback such that we can further improve it.)

kcrisman commented 10 years ago

comment:43

I just hate that someone is directed first to install something else in order to even start thinking of developing Sage. :-(

And it's IMHO a bit weird to assume someone would start developing Sage without (ever?) having it installed...

Well, what I meant was installing something that isn't Sage itself, sorry if that wasn't clear.

sagemath / sage

Use "git trac" in the developer guide #16030