Shell Script Cleanup - Githubissues

adorsys / xs2a

Open Source NextGenPSD2 XS2A Implementation from adorsys.

https://adorsys.com/en/products/

GNU Affero General Public License v3.0

130 stars 62 forks source link

Shell Script Cleanup #64

Closed Androbin closed 4 years ago

Androbin commented 4 years ago

Use $(...) notation instead of legacy backticked `...`.
Declare and assign separately to avoid masking return values.
Use cd ... || exit or cd ... || return in case cd fails.
which is non-standard. Use builtin command -v instead.
Quote this to prevent word splitting.
Useless echo? Instead of echo $(cmd), just use cmd.
Double quote to prevent globbing and word splitting.

Androbin commented 4 years ago

The openjdk11 build passed, is the openjdk8 build flaky?

DG0lden commented 4 years ago

openjdk8 build works. Seems your branch was created before the token to Sonarcloud was added. Please try to merge differences in .travis.yml into your file.

Androbin commented 4 years ago

As it turns out, MAVEN_OPTS is a string that really should be an array. I reverted the relevant change and left a TODO comment instead.

Androbin commented 4 years ago

@DG0lden Are you sure that the token is available to builds triggered by Pull Requests? I have seen cases where they were configured differently (for security purposes, so you couldn't print the token), which effectively meant that certain test would never work on PR, even if they do after being merged into master.

DG0lden commented 4 years ago

@Androbin you are right, secret variables are not populated to PRs

elroman commented 4 years ago

Thanks! Your changes were merged step-by-step to avoid conflicts. So, this pull request could be closed.