[riot-notifications] [RIOT-OS/RIOT] tools/ci: correcly report flake8 version. (#10252)

Joakim NohlgÄrd notifications at github.com
Fri Jan 18 21:06:44 CET 2019


gebart commented on this pull request.

See comments, untested though

> @@ -5,21 +5,17 @@ get_cmd_version() {
         return
     fi
 
-    local cmd="$1"
-    if command -v "$cmd" 2>&1 >/dev/null; then
-        ver=$("$cmd" --version 2> /dev/null | head -n 1)
-        # some tools (eg. openocd) print version info to stderr
-        if [ -z "$ver" ]; then
-            ver=$("$cmd" --version 2>&1 | head -n 1)
-        fi
-        if [ -z "$ver" ]; then
-            ver="error"
-        fi
-    else
-        ver="missing"
+    VERSION_RAW=$( ($@ --version) 2>&1)

Should this be `"$@"` instead of `$@`?
(combined with removing the quotes below at the get_cmd_version call for flake8
```suggestion
    VERSION_RAW=$( ("$@" --version) 2>&1)
```

> @@ -5,21 +5,17 @@ get_cmd_version() {
         return
     fi
 
-    local cmd="$1"
-    if command -v "$cmd" 2>&1 >/dev/null; then
-        ver=$("$cmd" --version 2> /dev/null | head -n 1)
-        # some tools (eg. openocd) print version info to stderr
-        if [ -z "$ver" ]; then
-            ver=$("$cmd" --version 2>&1 | head -n 1)
-        fi
-        if [ -z "$ver" ]; then
-            ver="error"
-        fi
-    else
-        ver="missing"
+    VERSION_RAW=$( ($@ --version) 2>&1)
+    ERR=$?
+    VERSION=$(echo "$VERSION_RAW" | head -n 1)

it is more reliable to use `printf %s "$VERSION_RAW"` instead of echo to avoid getting strange outputs in certain situations, especially since `$VERSION_RAW` may contain anything.

>  done
+printf "%23s: %s\n" "flake8" "$(get_cmd_version "python3 -Wignore -m flake8")"

```suggestion
printf "%23s: %s\n" "flake8" "$(get_cmd_version python3 -Wignore -m flake8)"
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/10252#pullrequestreview-194259884
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190118/6889e847/attachment.html>


More information about the notifications mailing list