Development strategy

Hi everyone,

[For those uninitiated, this is about my GSoC project about adding caching to Gitweb, on which I am currently working full-time; my branch is here: http://repo.or.cz/w/git/gitweb-caching.git]

I'm seeing a process-wise problem arising with my current work on Git.pm and Gitweb, in that I'm producing patches at a higher frequency than the developer community can handle. Right now, I have a stack of 7 patches that haven't been "approved" in any way (i.e. either they are under discussion or nobody has reviewed them) and that my future work will depend on. Chronologically,

! 2f15087248 perl/Git.pm: add get_hash method
   abd799435c gitweb: use Git.pm, and use its get_hash method
! 5e53e2e67e t/test-lib.sh: add test_external
! c5c97f896c Git.pm: fix documentation of hash_object
! 8db2d73809 test suite for Git.pm
   261a54ff5b gitweb: removed unused parse_ref method [not posted yet]
   e9166526a3 Git.pm: add get_type method [not posted yet]

The patches marked "!" I cannot change without breaking at least one subsequent patch, so every time I want to make a change to one of those, I also need to change at least one subsequent commit, and worse, resubmit it to the mailing list. (E.g. the the recent rename from parse_rev to get_hash necessitated two extra patches to accommodate the change.)

If I keep on committing revisions like I'm doing right now, I'll end up with a long queue of interdependent patches of which only the newest few are easily changeable. Here are some ideas to prevent this:

1) My current patch frequency is probably too high, in particular if 
minor changes (like method renames or documentation changes) cause 
re-posts of patches.  Hence, I suggest that I stay on my branch for now 
and only post patches on which I actually want feedback, without 
reposting corrected versions after getting feedback.  That is, no 
patches just for "please someone approve this".

2) I should probably also try to squash patches into larger ones.  This 
will make it easier to make changes in older commits, since I don't have 
to edit several commits, and it reduces the probability of merge 
conflicts.  I'm not sure how much squashing is appropriate though: For 
example, would it be okay to squash a fix like "Git.pm: fix 
documentation of hash_object" into a larger "Git.pm: add and fix several 
methods" commit, where I only mention the documentation-fix in the log 
message?  It would certainly be helpful in that it reduces the number of 
conflicts, but it will make the logs coarser.

-- Lea

On Mon, Jun 2, 2008 at 5:51 PM, Lea Wiemann <lewiemann@gmail.com> wrote:

Why not go for the win and do both? Keep your commit strategy as is, and create a branch whenever you want review, say review-02-06 or such and in that branch you squash the commits as appropriate (perhaps StGit can help here). These patches are mostly not intended for inclusion just yet (but for review only), right? Instead of sending all the small commits you send a bigger one, perhaps with a link to where the branch can be found for easy checkout. You can keep the branch for the a record or delete it after the review and go on with the next branch.

Cheers,

Sverre, who thinks a few RFC patches are in place soon for git-stats too and is glad you are doing the fine-tuning wrt patch submission.

"Sverre Rabbelier" <alturin@gmail.com> writes:

By the way, do you have public git repository set up somewhere (for example at repo.or.cz)?

Shawn, what is the status of other git GSoC 2008 projects?

On Mon, Jun 2, 2008 at 9:09 PM, Jakub Narebski <jnareb@gmail.com> wrote:

Yes I do :). http://repo.or.cz/w/git-stats.git I will put up a link at http://alturin.googlepages.com/gsoc2008.

Hi,

On Mon, 2 Jun 2008, Jakub Narebski wrote:

Let's not ask Shawn who is overloaded with work already, but the mentors/students.

As for my student, Miklos, who is pretty close to submitting builtin-merge rc1 (to be held off until after 1.5.6), he has

	http://repo.or.cz/w/git/vmiklos.git

Ciao, Dscho

I don't think he is informed about every project. ;-)

Well, I -- the one who does git-sequencer -- am currently on making a first prototype (according to a first spec) work/clean ;)

I have a repo at repo.or.cz, but have not yet pushed to it because my compromised Debian SSH key has still access to repo.or.cz and my new one hasn't.

Regards,
  Stephan

Hi,

On Mon, 2 Jun 2008, Stephan Beyer wrote:

Why don't you just remove your old account from that repository, add a new account with the new keys, and use that, until Pasky finally integrated the changes to repo.git to change your keys?

Ciao, Dscho

Sverre Rabbelier wrote:

Are you suggesting that the squashed patches get merged, or that the squashed patches get reviewed but the finer-granulated patches get merged? In the former case, I'd probably prefer to work with larger patches in the first place (and not just squash them on the review branch), since they are easier to handle -- e.g. I sometimes need to go back and change things in earlier commits, and in those cases larger commits are easier.

-- Lea

On Tue, Jun 3, 2008 at 12:36 AM, Lea Wiemann <lewiemann@gmail.com> wrote:

The latter is what I suggested :). I reckon you'll want ot keep the finger-granulated history for future reference / bisecting, but when reviewing that seems overkill.

If you don't think you're omitting information you will want later on (meaning the more specific commit history), by all means, go with the bigger patches. I find it nicer to create small commits, with the plus that it cuts down the size of the commit msg too ;).

Lea Wiemann <lewiemann@gmail.com> writes:

If the need to cascade changes is about something minor like naming, do not be too picky while you are presenting the progress. I personally think the method for getting the output from rev-parse should be named rev_parse and not get_hash, if only not to confuse plumbing users, but do not scramble and rebase -i many patches only because I said so today.

That will be a waste of your time. Something small like that, I do not think anybody would mind if at the very end of the series there is a patch that modifies all users of get_hash to use rev_parse and rename the method. I certainly wouldn't.

If on the other hand the issue is about a design mistake you had in earlier patches in the series, well, you may have to re-roll the later ones to adjust to the interface change _if_ you want to eradicate the design mistake from the history. But this too depends on the nature of the mistake.

For example, you may introduce a new sub "foo" in commit #1 to support a small feature or two you add in commit #2 and #3. Later, you would want add yet another new thing that can use something very close to what "foo" already does, but realize that you need to call "foo" slightly differently by giving an additional parameter.

You might wish that you had a perfect foresight to have that extra parameter from the beginning. But _nobody has perfect foresight_. I would not want you to start amending from commit #1 in such a case. Just make commit #4 to be "Enhance 'foo' to deal with this new situation", and that commit will change the implementation of "foo" and add the necessary extra parameter to the existing calling site you introduced in commit #2 and #3. Then commit #5 will use that new interface to introduce the new feature you needed an enhanced "foo" and you go on. Such a case of "the helper introduced in earlier round was too limited and it is later found that it needs enhancement" is not even a mistake you would want to remove from the history. It is just a normal course of code evolution.

Keep your commits small, logically independent steps.

Do not _artificially_ squash the commits --- it is _much worse_ than following up with enhancement patches. I think your current patch granularity is quite fine (in good/bad sense not in big/small sense) and makes pleasant read.