From 1e0d0395111033453c865a79eda1e6cda7ba3348 Mon Sep 17 00:00:00 2001
From: Aneesh Agrawal <aneeshusa@gmail.com>
Date: Sat, 30 Apr 2016 23:29:06 -0400
Subject: [PATCH 1/2] Clean up CI bash scripts, add docs/STYLE_GUIDE.md

---
 docs/STYLE_GUIDE.md        | 29 +++++++++++++++++++++++++++++
 etc/ci/check_no_unwrap.sh  |  5 +++--
 etc/ci/lockfile_changed.sh |  9 +++++++--
 etc/ci/manifest_changed.sh |  5 +++--
 etc/ci/retry.sh            | 16 +++++++++-------
 etc/ci/upload_docs.sh      |  8 +++++---
 6 files changed, 56 insertions(+), 16 deletions(-)
 create mode 100644 docs/STYLE_GUIDE.md

diff --git a/docs/STYLE_GUIDE.md b/docs/STYLE_GUIDE.md
new file mode 100644
index 00000000000..b7a3e519b42
--- /dev/null
+++ b/docs/STYLE_GUIDE.md
@@ -0,0 +1,29 @@
+# Style Guide
+
+The majority of our style recommendations are automatically enforced via our
+automated linters. This document has guidelines that are less easy to lint for.
+
+## Shell scripts
+
+Shell scripts are OK for small tasks or wrappers, but prefer to use Python for
+anything with a hint of complexity or in general.
+
+Shell scripts should be written against bash, starting with this shebang:
+```
+#!/usr/bin/env bash
+```
+
+Note that the version of bash available on OS X by default is quite old, so be
+careful when using new features.
+
+Scripts should enable a few options at the top for robustness:
+```
+set -o errexit
+set -o nounset
+set -o pipefail
+```
+
+Quote all variables, using the full form: `"${SOME_VARIABLE}"`.
+
+Use `"$(some-command)"` instead of backticks for command substitution. Note
+that these should be quoted as well.
diff --git a/etc/ci/check_no_unwrap.sh b/etc/ci/check_no_unwrap.sh
index c2a35d06674..9f07b56aaf7 100755
--- a/etc/ci/check_no_unwrap.sh
+++ b/etc/ci/check_no_unwrap.sh
@@ -1,11 +1,12 @@
-#!/bin/bash
+#!/usr/bin/env bash
 #
 # Make sure listed files do not contain "unwrap"
+
 set -o errexit
 set -o nounset
 set -o pipefail
 
-cd $(git rev-parse --show-toplevel) # cd into repo root so make sure paths works in any case
+cd "$(git rev-parse --show-toplevel)" # cd into repo root so make sure paths works in any case
 
 # files that should not contain "unwrap"
 FILES=("components/compositing/compositor.rs"
diff --git a/etc/ci/lockfile_changed.sh b/etc/ci/lockfile_changed.sh
index 7aa1ccccfca..e89e044f375 100755
--- a/etc/ci/lockfile_changed.sh
+++ b/etc/ci/lockfile_changed.sh
@@ -1,4 +1,9 @@
-#!/bin/bash
-diff=$(git diff -- */*/Cargo.lock)
+#!/usr/bin/env bash
+
+set -o errexit
+set -o nounset
+set -o pipefail
+
+diff="$(git diff -- */*/Cargo.lock)"
 echo "$diff"
 [[ ! $diff ]]
diff --git a/etc/ci/manifest_changed.sh b/etc/ci/manifest_changed.sh
index 4f84e3d9acc..31db4474099 100755
--- a/etc/ci/manifest_changed.sh
+++ b/etc/ci/manifest_changed.sh
@@ -1,4 +1,5 @@
-#!/bin/bash
+#!/usr/bin/env bash
+
 set -o errexit
 set -o nounset
 set -o pipefail
@@ -11,6 +12,6 @@ set -o pipefail
 # Adding "--binary=" to skip looking for a compiled servo binary.
 ./mach test-wpt --manifest-update --binary= SKIP_TESTS > /dev/null
 
-diff=$(git diff -- tests/*/MANIFEST.json)
+diff="$(git diff -- tests/*/MANIFEST.json)"
 echo "$diff"
 [[ ! $diff ]]
diff --git a/etc/ci/retry.sh b/etc/ci/retry.sh
index 4c036085108..ea0e2897c1d 100755
--- a/etc/ci/retry.sh
+++ b/etc/ci/retry.sh
@@ -1,17 +1,19 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
 # Retries a given command until it passes
 # Run as `retry.sh N command args...`
 # where `N` is the maximum  number of tries, and `command args...` is the
 # command to run, with arguments
 
-n=$1
+set -o errexit
+set -o nounset
+set -o pipefail
+
+n="$1"
 shift; # this removes the first argument from $@
-for i in `seq $n`; do
+for i in $(seq $n); do
         echo "====== RUN NUMBER: $i ======";
-        if $@ # run command, check if exit code is zero
-        then
-                exit 0 # command passed, all is well
-        fi
+        # Run command and exit success if return code is 0, else ignore it
+        "$@" && exit 0 || true
 done
 exit 1
diff --git a/etc/ci/upload_docs.sh b/etc/ci/upload_docs.sh
index aac1374ceda..980032a1350 100755
--- a/etc/ci/upload_docs.sh
+++ b/etc/ci/upload_docs.sh
@@ -1,10 +1,12 @@
-#!/bin/bash
+#!/usr/bin/env bash
 #
 # Helper script to upload docs to doc.servo.org.
 # Requires ghp-import (from pip)
 # GitHub API token must be passed in environment var TOKEN
 
-set -e
+set -o errexit
+set -o nounset
+set -o pipefail
 
 cd "$(dirname $0)/../.."
 
@@ -15,4 +17,4 @@ cp etc/doc.servo.org/* target/doc/
 python components/style/properties/build.py servo html
 
 ghp-import -n target/doc
-git push -qf https://${TOKEN}@github.com/servo/doc.servo.org.git gh-pages
+git push -qf "https://${TOKEN}@github.com/servo/doc.servo.org.git" gh-pages

From cca3f7a1058e401a085a14030f7e61db9653b964 Mon Sep 17 00:00:00 2001
From: Aneesh Agrawal <aneeshusa@gmail.com>
Date: Sat, 30 Apr 2016 23:29:43 -0400
Subject: [PATCH 2/2] Remove etc/ci/retry.sh

It was added with the intention of being used on the CI,
but it was never added to the CI. We have better ways of finding
test failures and intermittents these days.
---
 etc/ci/retry.sh | 19 -------------------
 1 file changed, 19 deletions(-)
 delete mode 100755 etc/ci/retry.sh

diff --git a/etc/ci/retry.sh b/etc/ci/retry.sh
deleted file mode 100755
index ea0e2897c1d..00000000000
--- a/etc/ci/retry.sh
+++ /dev/null
@@ -1,19 +0,0 @@
-#!/usr/bin/env bash
-
-# Retries a given command until it passes
-# Run as `retry.sh N command args...`
-# where `N` is the maximum  number of tries, and `command args...` is the
-# command to run, with arguments
-
-set -o errexit
-set -o nounset
-set -o pipefail
-
-n="$1"
-shift; # this removes the first argument from $@
-for i in $(seq $n); do
-        echo "====== RUN NUMBER: $i ======";
-        # Run command and exit success if return code is 0, else ignore it
-        "$@" && exit 0 || true
-done
-exit 1