Re: Shell script cleanups/style changes?

From: Bradford Smith <bradford.carl.smith@gmail.com>
Date: Aug 2, 2007, 14:48 UTC
Message-ID: <f158199e0708020748o5cac4505ic76bfb39f25ab1f8@mail.gmail.com>
In-Reply-To: <86bqdqkygp.fsf@lola.quinscape.zz>

On 8/2/07, David Kastrup <dak@gnu.org> wrote:

Show 14 quoted lines

> Hi, I wanted to ask what the general stance towards shell script
> cleanups and simplifications would be.  For example, I find the expr
> usage quite inscrutable in commit, and there is no necessity of
> putting "shift" in every case branch instead of once behind it, and a
> lot of conditionals and other manipulations can be made much easier on
> the eye by using parameter expansion patterns that are, as far as I
> can see, available with every reasonable Bourne Shell and clones.
>
> Here is an example context diff (in this case, I find it more readable
> than unified) to illustrate (untested!, please don't apply without a
> regular formatted git patch).
>
> Should I bother doing such cleanups as I read up on code, or should I
> just leave things alone?

I have no authority over the git project, but please consider this argument:

Every time you submit a patch there are three costs:
1. The time you put into making the patch.
2. The time required for the maintainer to review the patch and
possibly merge it into the code base.
3. The risk that you may have accidentally broken something.

Obviously, you aren't too concerned about 1 (the cost to you), because you're willing to do that work. However, if I were Junio, I wouldn't be willing to "spend" costs 2 and 3 on a patch that didn't either fix a problem or provide a new feature.

So, I recommend you do the clean-up that you want to do on your own local branch. This will no doubt be fun and educational for you. I know I've learned a lot in the past by experimentally "cleaning up" old ugly code on other projects, even though the result never made it into the official code base.

Along the way, you will probably find real bugs. When you do, submit patches for them based on the current master branch. You can probably manage to sneak a bit of clean-up into those bug-fixing patches, as long as you make sure it is all relevant to fixing the bugs and you keep the patches readable.

Best Wishes,

Bradford C Smith

p.s. I should also point out that writing portable shell scripts is far from trivial, so it is very difficult to be certain that what works for you will work for someone with a different shell.