Shellcheck pass on web-add.sh and --gid fix

Ghost commented

2018-10-25 17:44:59 +02:00

First-time contributor

Please review! You're probably more familiar with the code than I am.

benpro was assigned by Ghost

2018-10-25 17:50:11 +02:00

benpro commented

2018-10-26 09:49:10 +02:00

Contributor

LGTM but did you try the script with your patch? Is it working well? Nothing broken?
This is a big commit and not atomic, not easy to review.

LGTM but did you try the script with your patch? Is it working well? Nothing broken? This is a big commit and not atomic, not easy to review.

Ghost commented

2018-10-26 15:17:07 +02:00

Author

First-time contributor

Are you looking at the unified diff or the individual commits? I
tried to make them as atomic as I could, but yes I'll have to be
more careful in the future.

I've mostly tested vhost creation /
deletion, as well as aliases. And have tested some specific syntax
changes in the shell.

#!/bin/env bash
echo ${in_uid:+'--uid' "$in_uid"}
in_uid=39
echo ${in_uid:+'--uid' "$in_uid"}

Are you looking at the unified diff or the individual commits? I tried to make them as atomic as I could, but yes I'll have to be more careful in the future. I've mostly tested vhost creation / deletion, as well as aliases. And have tested some specific syntax changes in the shell. ``` #!/bin/env bash echo ${in_uid:+'--uid' "$in_uid"} in_uid=39 echo ${in_uid:+'--uid' "$in_uid"} ```

benpro commented

2018-10-29 10:06:20 +01:00

Contributor

Are you looking at the unified diff or the individual commits? I tried to make them as atomic as I could, but yes I’ll have to be more careful in the future.

The unified diff a.k.a the full merge request.

> Are you looking at the unified diff or the individual commits? I tried to make them as atomic as I could, but yes I’ll have to be more careful in the future. The unified diff a.k.a the full merge request.

Ghost commented

2018-10-29 15:14:45 +01:00

Author

First-time contributor

The next script I review I'll keep it to one function per pull request if you want. Instead of one commit per shellcheck rule with a unified pull request. These types of linting passes tend to be a bit messy by nature.