On Mon, May 21, 2012 at 10:18:39AM +0200, Janne Grunau wrote:
> On 2012-05-18 19:05:05 +0200, Diego Biurrun wrote:
> > --- a/doc/git-howto.texi
> > +++ b/doc/git-howto.texi
> > @@ -346,6 +346,58 @@ git checkout -b svn_23456 $SHA1
> >
> > +@chapter pre-push checklist
> > +
> > +Once you have a set of commits that you feel are ready for pushing,
> > +work through the following checklist to doublecheck everything is in
> > +proper order.
>
> I honestly think the wording and the checklist is way too strict for
> most commits. Strictly following it for every commit is nonsense and
> I'd guess nobody will do it.
Really? I do it for all batches of commits that I push. Now I certainly
do not mind common sense being applied, but such a checklist should cover
all bases and not teach you how to cut corners from the outset.
> > +First make sure your Git repository is on a branch that is a direct
> > +descendant of the Libav master branch, which is the only one from which
> > +pushing to Libav is possible. Then run the following commands:
> > +
> > +@itemize
> > +@item @command{git cherry -v}
> > +
> > +to make sure that only the commits you want to push are pending.
> > +
> > +@item @command{git log}
> > +
> > +to check that the log messages of the commits are correct and descriptive
> > +and contain no cruft from @command{git am}.
>
> why not combining these two with @command{git log origin/master..}?
Good idea.
> > +@item @command{git log -p}
> > +
> > +to doublecheck that the commits you want to push really only contain the
> > +changes they are supposed to contain.
>
> --stat might be helpful too, add the same 'origin/master..', mention
> that it coud be combined with the above.
Good idea as well.
> > +@item @command{git status}
> > +
> > +to ensure no local changes still need to be committed and that no local
> > +changes may have thrown off the results of your testing.
> > +@end itemize
> > +
> > +Next let the code pass through a full run of our testsuite. Before you do,
> > +the command @command{make fate-rsync} will update the test samples. Changes
> > +to the samples set are not very common, so updating it once per day or so
> > +is sufficient. Now execute
>
> Updating once per day is always enough since put new samples at 24h in
> place before pushing the
Good point, changed locally.
> > +@itemize
> > +@item @command{make distclean}
>
> When is a make clean not enough?
>
> > +@item @command{./configure}
>
> without distclean this could be a simple make config which would handle
> out of source tree builds and other required configure options just
> fine.
Supposedly 'make distclean' returns you to a vanilla source tree. I like
it because it forces you to do a fresh configure run and stray options
you might have in there will not throw off your test results.
In theory there could always be bugs in the dependency system that can
cover up issues. In practice I think we have had bugs that only showed
after a distclean, even though I cannot name any off the top of my hat.
> > +@item @command{make check}
> > +@end itemize
> > +
> > +While the test suite covers a wide range of possible problems, it is not
> > +a panacea. Do not hesitate to perform any tests necessary to convince
> > +yourself the changes you are about to push actually work as expected.
>
> you fail to mention that every commit is expected to pass make check
You mean every single commit or the fact that errors from "make check"
should not be ignored and commits pushed regardless?
> > +Finally, after pushing, mark all patches as committed on
> > +@url{http://patches.libav.org/ patchwork}.
> > +Sometimes this is not automatically done when a patch has been
> > +slightly modified from the version on the mailing list.
>
> git push reports for every commit if it could match a patch. Updating
> not revisions of the patch on patchwork is also a good idea if not
> done earlier.
"not revisions"?
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
