[services] Clean up `proto_codegen.sh` using ShellCheck
ClosedPublic
Actions

Authored by • abosh on Aug 2 2022, 10:06 AM.

Details

Reviewers

atul
• jon
tomek
ashoat

Commits

rCOMM6a9572b06930: [services] Clean up `proto_codegen.sh` using ShellCheck

Summary

Related Linear issue here. This is part of a set of diffs that will allow ShellCheck to be added to the CI. See inline comments for specific details of the ShellCheck error/warning output.

Test Plan

ShellCheck

Additionally, this script is run by build_service.sh, which is run during the Backup, Blob, and Tunnelbroker builds so if those workflows continue to pass CI we can reasonably assume that the script continues to work as expected.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• abosh created this revision.Aug 2 2022, 10:06 AM

Herald added subscribers: • karol, • adrian. · View Herald TranscriptAug 2 2022, 10:06 AM

Harbormaster returned this revision to the author for changes because remote builds failed.Aug 2 2022, 10:06 AM

Harbormaster failed remote builds in B11064: Diff 15229!

Remove extraneous protos/ on line 8, since now that we are not using ls to loop through the protos directory we do not need to include it as a parent directory. Noticed this because the CI was failing, re-tested the Blob build CI locally and it worked with no errors.

• abosh edited the test plan for this revision. (Show Details)Aug 2 2022, 10:43 AM

Harbormaster completed remote builds in B11065: Diff 15231.Aug 2 2022, 10:52 AM

• abosh requested review of this revision.Aug 2 2022, 10:52 AM

• abosh added inline comments.Aug 2 2022, 10:54 AM

services/lib/docker/proto_codegen.sh
7 ↗	(On Diff #15231)	(error): Iterating over ls output is fragile. Use globs.
8 ↗	(On Diff #15231)	(warning): Quote this to prevent word splitting. Use $(...) notation instead of legacy backticked ...
8 ↗	(On Diff #15231)	Double quote to prevent globbing and word splitting.