Some inline comments, also, commit b2e5f39 is not sufficient to add pre-commit to the LPCI pipeline. Please see: https://git.launchpad.net/autopkgtest-cloud/tree/.launchpad.yaml
For how to do this. Diff comments: > diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml > new file mode 100644 > index 0000000..7bff89d > --- /dev/null > +++ b/.pre-commit-config.yaml > @@ -0,0 +1,27 @@ > +--- > +repos: > + - repo: https://github.com/adrienverge/yamllint.git > + rev: v1.29.0 > + hooks: > + - id: yamllint > + args: [--strict] > + - repo: https://github.com/adrienverge/yamllint.git > + rev: v1.29.0 > + hooks: > + - id: yamllint > + alias: yamllint-jobs > + name: yamllint (for jobs.yaml files) > + args: [--strict, -c, .yamllint-jobs] > + - repo: https://github.com/shellcheck-py/shellcheck-py > + rev: v0.9.0.5 > + hooks: > + - id: shellcheck > + args: ['-e', 'SC1090', '-e', 'SC1091', '-e', 'SC2001'] Since we used a configfile for yamllint, let's also use a config file for shellcheck. Details can be found in this commit: https://github.com/koalaman/shellcheck/commit/581bcc3907ab98e919a7dd60566810a928c46b95 I'm not sure if it'll be available in the pre-commit hook, but let's try anyway :) > + - repo: local > + hooks: > + - id: deploy-jobs > + name: deploy-jobs > + entry: scripts/deploy-jobs.sh > + args: > + - "test" > + language: script > diff --git a/.yamllint-jobs b/.yamllint-jobs > new file mode 100644 > index 0000000..45421e4 > --- /dev/null > +++ b/.yamllint-jobs > @@ -0,0 +1,12 @@ > +--- > +extends: default > + > +ignore: > + - "**/*.yaml" > + - "!**/*jobs*.yaml" > + - "!jobs/macros.yaml" > + > +rules: > + indentation: disable > + line-length: disable > + document-start: disable Is there a specific reason this rule is disabled? The other two make total sense, though I'm not sure with this one. > \ No newline at end of file > diff --git a/scripts/flash-device.sh b/scripts/flash-device.sh > index 1efa0db..9dbbbb8 100755 > --- a/scripts/flash-device.sh > +++ b/scripts/flash-device.sh > @@ -74,67 +76,67 @@ echo "Wifi: $WIFI" > echo "Fastboot: $FASTBOOT" > echo "Recovery: $RECOVERY" > QUERY_CMD="ubuntu-device-flash $SERVER $REVARGS query --channel=$CHANNEL > --device $DEVICE --show-image" > -echo $QUERY_CMD > +echo "$QUERY_CMD" > $QUERY_CMD > > rootcmd () { > - adb -s $ANDROID_SERIAL shell "echo $PASSWD | sudo -S $*" > + adb -s "$ANDROID_SERIAL" shell "echo $PASSWD | sudo -S $*" > } > > # Handle fastboot mode and fastboot recovery mode > if [ x"$FASTBOOT_RECOVERY" != x ]; then > echo 'Rebooting to bootloader/fastboot' > - adb -s $ANDROID_SERIAL reboot bootloader > + adb -s "$ANDROID_SERIAL" reboot bootloader > sleep 10 > echo "Flashing recovery image $FASTBOOT_RECOVERY" > - fastboot -s $ANDROID_SERIAL flash recovery $FASTBOOT_RECOVERY > + fastboot -s "$ANDROID_SERIAL" flash recovery "$FASTBOOT_RECOVERY" > echo "Rebooting into recovery" > - fastboot -s $ANDROID_SERIAL boot $FASTBOOT_RECOVERY > + fastboot -s "$ANDROID_SERIAL" boot "$FASTBOOT_RECOVERY" > sleep 10 > FASTBOOT=no > -elif [ x"$FASTBOOT" = xyes ]; then > +elif [ "$FASTBOOT" = xyes ]; then x seems to be removed here which may break the if statement? Though there may be a reason for this I am unaware of :) > EXTRA="--bootstrap" > echo 'Rebooting to bootloader/fastboot' > - adb -s $ANDROID_SERIAL reboot bootloader > + adb -s "$ANDROID_SERIAL" reboot bootloader > sleep 10 > fi > > echo 'flashing device' > FLASH_CMD="ubuntu-device-flash $SERVER $REVARGS touch > --serial=$ANDROID_SERIAL --channel=$CHANNEL --device $DEVICE $DEVMODE > $RECOVERY --wipe $EXTRA" > -echo $FLASH_CMD > +echo "$FLASH_CMD" > $FLASH_CMD > > echo 'waiting for device to boot' > -adb -s $ANDROID_SERIAL wait-for-device > +adb -s "$ANDROID_SERIAL" wait-for-device > echo 'waiting a few seconds for boot' > sleep 5 > > # setup network; location of config per plars' advice > DEFAULT_NETWORK_CONFIG_PATH="/var/lib/jenkins/.ubuntu-ci/wifi.conf" > -if [ -n NETWORK_CONFIG_PATH ]; then > +if [ -n "$NETWORK_CONFIG_PATH" ]; then > NETWORK_CONFIG_PATH=$DEFAULT_NETWORK_CONFIG_PATH > fi > -phablet-network -n $NETWORK_CONFIG_PATH > +phablet-network -n "$NETWORK_CONFIG_PATH" > > # Disable the welcome wizard > -if [ x"$WIZARD" = x"no" ]; then > +if [ "$WIZARD" = x"no" ]; then same here, x is removed > echo 'disabling wizard' > - phablet-config -s $ANDROID_SERIAL welcome-wizard --disable > - adb -s $ANDROID_SERIAL wait-for-device > + phablet-config -s "$ANDROID_SERIAL" welcome-wizard --disable > + adb -s "$ANDROID_SERIAL" wait-for-device > REBOOT_NEEDED=yes > fi > > # Disable the edges intro > -if [ x"$EDGEINTRO" = x"no" ]; then > +if [ "$EDGEINTRO" = x"no" ]; then x also removed here? > echo 'disabling edges intro' > - phablet-config -s $ANDROID_SERIAL edges-intro --disable > - adb -s $ANDROID_SERIAL wait-for-device > + phablet-config -s "$ANDROID_SERIAL" edges-intro --disable > + adb -s "$ANDROID_SERIAL" wait-for-device > fi > > -if [ x"$REBOOT_NEEDED" = x"yes" ]; then > +if [ "$REBOOT_NEEDED" = x"yes" ]; then and the same here > echo 'rebooting to activate previous changes' > - adb -s $ANDROID_SERIAL reboot > - adb -s $ANDROID_SERIAL wait-for-device > + adb -s "$ANDROID_SERIAL" reboot > + adb -s "$ANDROID_SERIAL" wait-for-device > fi > > # wait until our network is ready -- https://code.launchpad.net/~canonical-platform-qa/qa-jenkins-jobs/+git/qa-jenkins-jobs/+merge/471372 Your team Canonical Platform QA Team is requested to review the proposed merge of qa-jenkins-jobs:pre-commit-hooks into qa-jenkins-jobs:master. -- Mailing list: https://launchpad.net/~canonical-ubuntu-qa Post to : canonical-ubuntu-qa@lists.launchpad.net Unsubscribe : https://launchpad.net/~canonical-ubuntu-qa More help : https://help.launchpad.net/ListHelp
