[arch-general] [PATCH 01/48] Bashification of initscripts

[Dan McGee]

On Tue, Jul 6, 2010 at 11:03 PM, Allan McRae <allan at archlinux.org> wrote:
> Here is a quick review on all these patches.   I recommend that the lvm and
> crypttab changes get a decent amount of testing before these go live as they
> are the biggest changes being done.
>
>  Why has this been removed:
>    -if [ -x /etc/rc.local.shutdown ]; then
>    - /etc/rc.local.shutdown
>    -fi
>  Ah... it has been moved to another place in another commit.  Please
> document these sorts of changes in your commit message and preferably do the
> entire move in one commit.

If there was one thing I wasn't so fond of in these patches, it seemed
like there were too many. The beauty of git is the ability to go back
and squash and split patches in a way that makes a lot more sense to
others- it might not have been the way or order you did things in, but
you should try as hard as possible to make a commit the largest
logical unit that makes sense, but still small enough to grasp fully.

If there are ever closely-related changes strewn across multiple
patches in a patch set, you should probably think about merging those
commits.

-Dan