[Koha-devel] koha code cleaning bug?

[Galen Charlton]

Hi,

On Mon, Sep 29, 2008 at 5:17 AM, Marc Chantreux
<marc.chantreux at biblibre.com> wrote:
> for('',qw( lock register shadow tab key )) {
>    next if -d;
>    $created_dir_or_file++;
>    system("mkdir -p $authorityserverdir");
>    print "Info: created $authorityserverdir/$_";
> }

If clarity is the goal, I would prefer

for my $subdir (qw( lock register shadow tab key)) {
  ...
}

i.e., please use an explicit loop variable instead of $_.  Otherwise,
I agree with rolling these up into a loop.

> That is not a bug but it is currently written makes the things harder to read
> and maintain and it discourages to provide a patch to improve the code(imho). I
> would like to provide a patch to clean this code.

To suggest some guidelines:

1. If you're doing this sort of stylistic change, please consider
writing test cases.
2. Unless you're revamping an entire module or script, don't mix
cleanup (that theoretically shouldn't change behavior) with
functionality changes in the same patch.
3. If you fix a problem in one place, consider fixing it in all places.
4. The goal of a refactoring should be to improve clarity,
maintainability, testability, or logical structure.  Please don't
"refactor" just to show off your knowledge of Perl arcana; a newbie to
Perl should have a fighting chance of understanding Koha's code.

> This code is not the only one and so i wonder if the koha communauty wants to
> have this kind of patch. I also wonder how to do this because of the "fill
> a bug first" policy. It can be possible to create a "code cleaning" bug that
> will stay unfixed and our patches can be related to this one.

In general, yes, I will accept cleanup patches.  Cleanup patches that
have test cases will be applied more quickly than those that lack
them.

A generic code-cleaning bug is too vague; I suggest opening a bug for
each cleanup, to remain open until all instances of that cleanup have
been completed.

Regards,

Galen
-- 
Galen Charlton
VP, Research & Development, LibLime
Koha 3.2 Release Manager
galen.charlton at liblime.com
p: 1-888-564-2457 x709
skype: gmcharlt