developer's guide: warnings & notes about commit message, cloning and setting the text editor

[sagetrac-mvngu]

Add a new chapter to the Developer's Guide on advanced uses of Mercurial.

See the following URL for the rebuilt Developer's Guide after applying the patch below to Sage 4.6.

http://mvngu.googlecode.com/hg/10190-mercurial/index.html

CC: @kcrisman @nexttime

Component: documentation

Keywords: Mercurial, developer's guide

Author: Minh Van Nguyen

Reviewer: Marc mezzarobba

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

[sagetrac-mvngu]

comment:1

The attachment provides the following changes to the Developer's Guide:

• Add a new section "Commit messages" to explain that you can have a commit message that spans multiple lines.
• Add a new section "Setting the editor" to explain how to set the editor that Mercurial launches.
• Cross reference between the walk-through and the chapter on advanced Mercurial usage.

[sagetrac-mvngu]

Author: Minh Van Nguyen

[nexttime]

comment:3

I think the description of commit messages is still somewhat misleading, especially the first time they are mentioned.

IMHO one should almost always give a "long[er]" commit message, but in any case the first line has to be one summarizing the whole patch.

(The example of Jeroen's long commit message might still confuse people, since it consists of a few lines all starting with a ticket number, all being "short" commit messages, i.e. summaries by themselves. For spkgs, I usually copy the whole changelog entry from SPKG.txt into the [final] commit message, below the summary and a blank line. Other commit messages look similar.)

Karl-Dieter, you can perhaps better comment on the changes regarding Mercurial's use of an editor to provide commit messages.

(To me, e.g. using Mercurial from the Sage prompt is more complicated than using it directly from the shell. One does not have to record changeset numbers, filename (and option) completion is available, not to mention help etc. ...)

[rbeezer]

comment:4

Replying to @nexttime:

I think the description of commit messages is still somewhat misleading, especially the first time they are mentioned.

I'm in the middle of reviewing this one. I wrote much of the walk-through. At one time I was given the impression a one-line message was best. Thus the strident "one-line" request.

I agree that a multi-line commit can be an improvement. Personally, I would go back to Trac if I wanted to know more. So I don't necessarily agree that they should be required or preferred. As a patch author, I've always found one line sufficient (and less work!).

What I have seen is rookie developers put gobs of stuff in the commit message, with no summary line. Not pretty. So a more thorough discussion in the advanced section is a good compromise I think. What would you suggest for the introductory discussion?

Re: editors,

hg qrefresh -m "#xxxx: Summary..."

is my optimal solution. But not good for multi-line stuff.

[nexttime]

comment:5

Replying to @rbeezer:

I agree that a multi-line commit can be an improvement. Personally, I would go back to Trac if I wanted to know more.

That's IMHO a mess. I don't want to connect to trac in the first place when trying to find out when and why somebody changed this or that. (And you know how some tickets look like.)

A few people name their patches just 6812.patch, and some put just the ticket number in the commit message.

It's already often tedious enough for spkgs to map changes to patch levels (and their changelog entries if at all present; many old ones are missing, others are less informative).

So I don't necessarily agree that they should be required or preferred.

At least welcome, if they contain useful information.

What I have seen is rookie developers put gobs of stuff in the commit message, with no summary line.

Yes, "... and also changed the third word bhfsdifugsdi on line 5628 (right after djshdgh) to ghjgjhgfdsj and then inserted 4 blank lines so that the line below starting with 'if (xy+2)<z' is now line 5633 ..."*

Not pretty. So a more thorough discussion in the advanced section is a good compromise I think. What would you suggest for the introductory discussion?

One shouldn't have to (perhaps by chance) read an advanced section to learn that longer commit messages aren't forbidden (but instead even useful)... The paragraph is quite long, but doesn't refer to longer commit messages at all.

A description of them with the commit message of this ticket's patch as an example in the advanced section is ok. ;-)

[nexttime]

comment:6

P.S.: I think it's a bit funny to describe Mercurial queues in the "introductory" section, but have a note on commit messages, and also how to set an editor, in an "advanced" section.

I suggested to put the editor = ... into the template, along with a small note that people not familiar with vi should change that to one they are used to (or read about vi) before they do their first commit. (The advanced section, or a "warning" block in the introduction, could then also describe how to enter text in vi and - especially - how to quit vi.)

[sagetrac-mvngu]

comment:7

The above reviews by leif and rbeezer are the sort of reviews I really want from contributors: constructive, to the point, what the patch lacks, possible ways to make the patch better, etc. I'll rework the patch and the walk-through chapter.

[rbeezer]

comment:8

Replying to @sagetrac-mvngu:

The above reviews by leif and rbeezer

Hi Minh and Leif,

I don't do as much system/build level work as leif, so his comments about spkgs, etc are important. However this changes, linking back to any advanced section is one way to keep the introduction short and make everything available to the person with more experience, or the newcomer who gains more experience.

When I start with newcomers (I've done this at least four times now), I just do queues and we never invoke the editor. But I like Leif's suggestion of including the editor command set up for vi and a comment in the template about making a change as an option.

I've had at least one suggestion from an experienced developer to make queues more prominent in the introduction and de-emphasise clones. I struggled with clones whenever I had to update a patch or wanted to add a reviewer patch, and I became so much more productive with queues. I don't think they are hard, or advanced. Being optional does not necessarily equate to being advanced.

There are two things I'd like to do long-term. A cleaner split between clones and queues, maybe even rearranging the order. I'd also like to do "queues under the hood" - a small discussion about the contents of .hg/patches as a way of understanding what is really happening. Of course that can all wait.

In a day or two I'll be totally busy with other stuff for a few weeks, even 100% offline for a week. Feel free to pester me about a review if it slips my mind once I'm back in the saddle (presuming no action in the next day or two).

Rob

[nexttime]

comment:9

Perhaps a bit OT: Is there any advantage (perhaps to a new user) of using Mercurial from the Sage prompt?

Since the section on queues directly uses hg from the shell, shouldn't we do that throughout the chapter, perhaps with a "footnote" that some functions are also available from the Sage prompt?

(If Sage's functions would abstract from Mercurial, they would make much more sense to me, but as is, they provide just a different, clumsy syntax for a very limited subset of Mercurial's features. Of course others might disagree on that.)

[sagetrac-mvngu]

comment:10

Updated patch takes into account most reviewer comments. I scrapped the idea of creating a new chapter on advanced Mercurial usage. That can wait for another ticket. The new patch has warnings and notes about commit message, cloning, and setting the text editor all within the walk-through chapter. Here's a summary of changes proposed by the patch:

• Warnings about the inefficiency of cloning and point out that Mercurial queues fits naturally into the patch management problem.
• Some typo fixes.
• Explain that you can write a commit message that spans multiple lines.
• Instructions on writing/editing a commit message with Vi(m) and then quit the Vi(m) editor.
• Explain that Sage's interface to Mercurial is very basic and that directly using Mercurial can allow you to be much more productive.

[rbeezer]

comment:11

Replying to @sagetrac-mvngu:

Hi Minh,

Thanks for your work on this one. Two quick comments before I head out for a bit.

1. When you say:

i.e. Mercurial queues, for efficiently working with a sandbox of 
the Sage library

I think it would be better not to infer that queues give you a "sandbox". A clone makes a copy of lots of source code and if you wreck it you can delete it. No harm. Queues work on the one copy of source code, though it is easy to back out of a mess, but you can't just trash it. Maybe delete "a sandbox of" for each copy of the warning?

2. I think leif's suggestion was to put an "editor=" line in the template, all filled in to point to vim. I have new users cut/paste the template to construct an .hgrc file - somebody wishing to change it would find it later in their .hgrc.

Gotta run.

Rob

[sagetrac-mvngu]

Attachment: trac-10190_mercurial.patch.gz

[sagetrac-mvngu]

comment:12

Replying to @rbeezer:

I think it would be better not to infer that queues give you a "sandbox". A clone makes a copy of lots of source code and if you wreck it you can delete it. No harm. Queues work on the one copy of source code, though it is easy to back out of a mess, but you can't just trash it. Maybe delete "a sandbox of" for each copy of the warning?

Done. The warnings are reworded to avoid giving the impression that queues give user a sandbox.

2. I think leif's suggestion was to put an "editor=" line in the template, all filled in to point to vim. I have new users cut/paste the template to construct an .hgrc file - somebody wishing to change it would find it later in their .hgrc.

Done. The updated ~/.hgrc template has the section editor = together with other common configuration options.

[kcrisman]

comment:13

Replying to @nexttime:

Perhaps a bit OT: Is there any advantage (perhaps to a new user) of using Mercurial from the Sage prompt?

Yes. Again, this comment can only come from someone who has used command line stuff for so long that it is second nature (that's not an insult, just a fact - please take no offense).

"Just" setting up an alias to Sage's HG is already non-intuitive, esp. if one doesn't know what a .bashrc or whatever is, and then there is the issue of when one upgrades/builds a new Sage, having to change the alias... or not setting up an alias, which is what I do in the rare instances that I need the full Sage HG.

But pointing out for those who have done so before or are comfortable with such things "here's how easy it is to set this up right" would be fine.

[nexttime]

comment:14

I was rather thinking that mixing the usage of Mercurial from the Sage prompt (a command line interface anyway ;-) but with a quite different syntax) and the shell prompt might be more confusing. (The Sage functions could be moved to another section or paragraph.)

Setting up an alias is a different issue; for those that don't have a system-wide hg we could use ./sage -hg ... (perhaps without the ./) in the examples.

[nexttime]

comment:15

Minh, in case I can consider your patch "stable", I'll create a patch on top of yours with some suggestions (but not right now), which I think is easier than listing them in a comment.

Just one point in advance: Wouldn't it be easier to start the examples from a Sage shell, ./sage -sh?

Then we'd not only have SAGE_ROOT a properly set environment variable, but also sage and other commands (including hg btw.) in the path ($PATH).

(I assume people more familiar with shells / the command line will be able to ignore this step and know how to "translate" the examples if necessary.)

[jhpalmieri]

comment:16

Here are some comments: on line 107 of the patch, "Mercurial queues fits" — change "fits" to "fit".

Lines 234-239: I don't agree that the Sage interface is limiting: you can execute any Mercurial command with the Sage library repo, for example, by typing hg_sage("command"). It is true that the number of options available via tab completion is not great, in particular missing anything having to do with queues.

I don't think that we need the same warning about cloning three different times, especially since the lines before give a link to the section on creating a clone, which includes the original warning. (I'm also hoping that #6495 will help speed up the cloning process.)

Regarding HGEDITOR etc., it seems to me that you don't need to enter the full path for the editor. I tried with export HGEDITOR=vim, and similarly for the editor setting in .hgrc, and it worked fine. Do we need so much detail for this setting, or can we just advise users to set their EDITOR environment variable, perhaps with a link to http://mercurial.selenic.com/wiki/editor?

[jhpalmieri]

comment:17

By the way, #11142 adds commands for working with queues to the Sage classes like hg_sage. Also, Leif, one small advantage of working within Sage is that you don't have to change to the appropriate directory first: you can start Sage from any directory and hg_sage... will access the main Sage repository. So you can work with all of the repos at once without changing directories or having multiple terminal windows open. Not a big deal, but one answer to your question.

[kcrisman]

comment:18

Replying to @jhpalmieri:

By the way, #11142 adds commands for working with queues to the Sage classes like hg_sage.

Oh, I didn't know that ticket did that too - that's great.

Also, Leif, one small advantage of working within Sage is that you don't have to change to the appropriate directory first: you can start Sage from any directory and hg_sage... will access the main Sage repository. So you can work with all of the repos at once without changing directories or having multiple terminal windows open. Not a big deal, but one answer to your question.

+1, I really like this part, though I'm starting to use hg outside sometimes now.

[mezzarobba]

comment:21

Probably no longer relevant.

[kcrisman]

comment:22

Yeah, though that doesn't mean that the git info doesn't need more details. But that would be a different ticket.

Usually for a "not relevant" you only need one reviewer + the release manager, so feel free to put positive review and your name.

[mezzarobba]

Reviewer: Marc mezzarobba