-
Notifications
You must be signed in to change notification settings - Fork 44
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
Cleanup shell scripts #54
Conversation
mpoullet
commented
Oct 31, 2018
•
edited
Loading
edited
- apply shellcheck and use Bash strict mode.
- use the same prolog in each shell script.
- avoid exporting variables when not needed.
- take advantage of git clean to cleanup.
- be less verbose with apt-get install.
- upgrade to gRPC v1.16.0 to address issue Unable to build googleapis in the latest version #35.
- fix language spelling.
Overall looks good. I'm going to run it through a CI server just to be safe. |
How was the CI run? |
It ended up failing. I've pasted the logs below.
|
I've triggered a CI build of this PR on AppVeyor using Ubuntu 18.04 and it works fine as you can see here and there. It works fine on my Ubuntu 16.04 too. However I can reproduce your issue with Travis CI on Ubuntu 14.04 too, see here. At the moment I'm stuck: gRPC v1.15.0 is needed to build the actual googleapis by providing protobuf v3.6.1 to avoid #35. gRPC v1.15.0 works on Ubuntu 18.04/16.04 but not 14.04. gRPC v1.12.0/v1.13.0/v1.14.0 are of no help as they all build protobuf version 3.5.2. I'd like to know how you can build Possible actions:
|
- use the same prolog in each shell script. - avoid exporting variables when not needed. - take advantage of git clean to cleanup. - be less verbose with apt-get install
I've narrowed/reproduced/reported the gRPC issue here. You might ask some fellow coworkers to have a look at it. |
Yeah if we revert back to 1.11.0, then we can't compile the protobufs correctly, unless we only select a subset of the files to compile. It would be okay to Then, we can go back to v1.15.0 later once issues there are resolved. |
It's not really an issue with |