On Feb 28, 2009, at 11:07 AM, Patrick R. Michaud wrote:
On Sat, Feb 28, 2009 at 10:46:10AM -0600, Andy Lester wrote:
On Feb 28, 2009, at 10:29 AM, Patrick R. Michaud wrote:
So, for the time being Rakudo's official policy will be to
accept patch submissions via RT. I've now cleared the fork queue
on GitHub; any patches that were previously there need to be
resubmitted.
My understanding is that this is bad for everyone else, because it
loses
the changeset history by making your committed patch different than
the
accumulated changesets that you're applying.
I suspect that "everyone else" should be making their changes in
personal branches as well, rather than expecting the rakudo/master
repository to exactly follow the commits in their forks.
I suspect there's a way of working the workflow that lets you easily
view them in aggregate. Perhaps the RT ticket would include not a
patch
but the commit IDs that the patch would include. The submitter says
<<END_OF_RT
"Hey, Patrick, I finished the work on the wangdoodle. Here are the
commits for it:
88b9da2ce43c4e8583cda28ace5144e0a618d70b
4a03ba2fe2e94895881d301de0446158424e7c1d
9e8c18ff0093125830a4bd06a3daee2e20cd8faa
0240f7776a9a01443567dfeaa98bba91b734affd
END_OF_RT
This approach goes totally the wrong way -- the submitter has to
somehow produce a (long) list of commits, which the maintainer then
has to manually replay in the review and test.
I'm sorry this is poorly researched. I'm heading out for the day, or
I'd have spent more time on it. I'm just concerned that using
patches
is very anti-git and will make things worse.
I agree we probably need more research here to potentially use
git/github more effectively, but what little I understand of the
fork queue absolutely doesn't work for me as a maintainer, and
turns the maintainers into a bottleneck.
The "send a patch via RT" approach is well understood, and has
worked fine for us for years. So, we either somehow improve our
understanding of git patch management (and believe me, I've
unsuccessfully looked for guides on "fork queue management for
maintainers"), or we stick with RT.
Pm
Based on my experience watching the Linux kernel development process
via git, I think both Andy and Patrick are partially right. An
important point is that git optimizes for scalability of the project
leadership.
Andy is right that it's enormously useful to preserve history. Git
has remarkable features where lieutenants can review and digitally
sign commits so the benevolent dictator can slurp them into HEAD
without needing to personally review the commits if he trusts his
lieutenants. Git also tracks the owner of a patch quite well, so
copyright concerns become more automatable. Linux cares deeply about
this because of cases like SCO where the project leaders want to know
who may have committed code of questionable origin.
But Patrick is right that reviewing false starts is an big waste of
time for project leaders, who may already be bottlenecks for a
project. The Linux dev process insists that submitters do this:
1) fork a trunk point
2) do work on a branch, which may include false starts
3) if the work is more than one commit:
a) make a new clean branch
b) manually rebuild the patch as a series of commits handling
separate concerns
c) ask upstream to pull that optimized branch
4) submitter deletes dev branch which included the false starts
This process means the submitted branch/patches represents a
fictitious optimal development, as if the programmer were perfect and
made no false starts along the way. That means more work for the
submitter of a complicated patch, but sometimes raising the bar is not
a bad thing.
On the other hand, if the submitter thinks the patch is awesome but
the reviewer thinks it needs a little work, that's a bit harder
because the reviewer needs to do one of the following:
1) branch and fix
a) make a branch
b) pull the submission
c) edit
d) commit
2) edit the patch as text and commit
3) reject the patch back to the submitter
Option #2 is the easiest but the worst because it loses attribution.
#3 happens the quite often in the Linux kernel process. We should not
view rejection as rude, but as necessary for a good code base.
The ffmpeg dev team takes a similar view on patch rejection -- because
video codecs are so dangerously patent-encumbered, the team won't
accept any patch until it's really clean and deeply understood. And
they have ended up with some surprisingly readable code as a result.
All that said, I have not studied the git best practices for reviewing
submissions. I strongly suspect that there's lots more we can learn
from Linux.
Chris
