Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some linter warnings #199

Closed
wants to merge 5 commits into from
Closed

Conversation

matrixik
Copy link
Member

@matrixik matrixik commented Sep 8, 2017

Some fixes for #191

@matrixik
Copy link
Member Author

matrixik commented Sep 8, 2017

Shellcheck is a little less noisy now.
And show some sh/bash specific problems like (we use #!/bin/sh everywhere):

In monasca-python/build.sh line 48:
    source /clone.sh
    ^-- SC2039: In POSIX sh, 'source' in place of '.' is undefined.
In monasca-python/build.sh line 52:
        local repo=$1
        ^-- SC2039: In POSIX sh, 'local' is undefined.
In mysql-init/init.sh line 139:
    if [ "$d_major" -le "$c_major" -a "$d_minor" -le "$c_minor" -a "$d_patch" -le "$c_patch" ]; then
                                   ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.
                                                                ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.

@matrixik matrixik changed the title Fix some linter errors Fix some linter warnings Sep 8, 2017
@timothyb89
Copy link
Member

Hmm. I think we're going to need to reevaluate some CI decisions. This last build took 7 hours (!) https://travis-ci.org/monasca/monasca-docker/builds/273282923?utm_source=github_status&utm_medium=notification

I'm guessing some networking issue was involved, but it really isn't properly debuggable given how noisy the logs are.

Short term this patch triggered too many container rebuilds, which takes more time and amplifies log output (monasca/dbuild#3). If we split this patch up to fix only a couple of modules at a time it should be able to pass CI.

Long term we need to fix dbuild's logging issue and figure out why these builds are taking so long and possibly also put a hard limit on the number of modules we'll build in a single patchset. I've filed #200 to track this.

FWIW this patch really isn't the cause of the problem but unfortunately I think we'll need to work around it while other things are cleaned up.

@matrixik
Copy link
Member Author

Ups, sorry for this. I'll split it to smaller PRs.

Hmmm, shouldn't person that open PR get mail from Travis that build failed?

@matrixik matrixik closed this Sep 11, 2017
@timothyb89
Copy link
Member

Yeah, you should have gotten a message... I guess we broke Travis in more ways than one 😃

@matrixik
Copy link
Member Author

Hmm, no mails with CI errors on my end from any PR...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants