Code review comment for ~vorlon/cloud-init:lp.1134036

Revision history for this message
Scott Moser (smoser) wrote :

First thought, a question:
  Would it be better to put this into base_files or even openssh-server ?
The scenario is not at all related to cloud-init. I'd be just as happy
to have cloud-init disable its behavior based on a fix present in another
package.

You need to fix the commit message.
lines should only be 74 chars wide.

I suggest for a subject line:
 Z99-cloud-locale-test: Fix invalid locale settings, do not print messages.

http://paste.ubuntu.com/p/s8dgFRnx43/

I've played around a bit and suggest the above.

Things I've changed there:

 * fix some complaints from shellcheck
 * make 'locale_fixup' take a default locale as param 1, using UTF-8.C as the default.
 * Avoided the double 'case' by just checking for empty 'val' or val that starts with '"'.
   I admit the primary reason for this is that 'vim' syntax highlighting didn't like
   the \"*\" that you had. My replaced version seems to be slightly faster, but
   that speedup is nominal compared to the subshells involved in $(locale | function)
 * I dropped the 'LANG' section because I can't figure out how to trip it.
   Also here the check for presense of 'bad' in 'bad_set' is now safer. if both
   LANG and LANGUAGE ever *did* make it into the 'bad_set', then yours would have
   LANG match LAUNGUAGE.
 * append to a variable and a single 'echo' rather than multiple lines in output.

« Back to merge proposal