Development strategy

Development strategy

[Lea Wiemann]

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

Re: Development strategy

[Sverre Rabbelier]

On Mon, Jun 2, 2008 at 5:51 PM, Lea Wiemann <lewiemann@gmail.com> wrote:
> 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.

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.

Re: Development strategy

[Jakub Narebski]

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

> 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.

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?
-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: Development strategy

[Sverre Rabbelier]

On Mon, Jun 2, 2008 at 9:09 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> By the way, do you have public git repository set up somewhere (for
> example at repo.or.cz)?

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

-- 
Cheers,

Sverre Rabbelier

Re: Development strategy

[Johannes Schindelin]

Hi,

On Mon, 2 Jun 2008, Jakub Narebski wrote:

> "Sverre Rabbelier" <alturin@gmail.com> writes:
> 
> > 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.
> 
> 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?

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

Re: Development strategy

[Stephan Beyer]

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

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

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

Re: Development strategy

[Johannes Schindelin]

Hi,

On Mon, 2 Jun 2008, Stephan Beyer wrote:

> 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.

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

Re: Development strategy

[Lea Wiemann]

Sverre Rabbelier wrote:
> Keep your commit strategy as is [with small commits],
> and create a branch whenever you want review and in that branch you
> squash the commits as appropriate

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

Re: Development strategy

[Sverre Rabbelier]

On Tue, Jun 3, 2008 at 12:36 AM, Lea Wiemann <lewiemann@gmail.com> wrote:
> Are you suggesting that the squashed patches get merged, or that the
> squashed patches get reviewed but the finer-granulated patches get merged?

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.

>  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.

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 ;).

-- 
Cheers,

Sverre Rabbelier

Re: Development strategy

[Junio C Hamano]

Lea Wiemann <lewiemann@gmail.com> writes:

> ! 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 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.