[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1033035: hw-detect: trivial patches



On 16/03/2023 at 12:05, Cyril Brulebois wrote:

Pascal Hambourg <pascal@plouf.fr.eu.org> (2023-03-16):

- check-missing-firmware.sh: shift positional parameters after reading
    -n

IIUC the script arguments are expected to be network interface names except the first one which may be "-n". In this case, I believe it should be shifted away before setting IFACES="$@".

- check-missing-firmware.sh: define local variables in functions

These variables are intended to be used only inside their respective functions, so it is cleaner and safer if they are declared as local (like in other hw-detect scripts) to avoid potential side effects with other functions or the main body if they use variables with the same names.

The commit messages say what you do, not why.

Sorry, I thought it was obvious.

- check-missing-firmware.sh: replace spaces with tabs in indentation

NACK. We have mixed tabs and spaces all over the place, in hw-detect and
in other components. We don't need noise. Especially not at this stage.

Spaces for indentation are present only in one function, so I thought it might be a text editor glitch or human error and it was desirable to make it consistent with the rest of the script. AFAICS the only consistent use of spaces for indentation in other hw-detect scripts is before "case" patterns, which makes sense. Most other occurrences look like mistakes. But I take your point about avoiding cosmetic fixes now.


Reply to: