Page MenuHomePhabricator

[services] Clean up `test_service.sh`
ClosedPublic

Authored by abosh on Jul 5 2022, 12:02 PM.
Tags
None
Referenced Files
F2194187: D4450.id14204.diff
Fri, Jul 5, 2:16 AM
Unknown Object (File)
Fri, Jun 21, 5:58 PM
Unknown Object (File)
Fri, Jun 21, 5:58 PM
Unknown Object (File)
Fri, Jun 21, 5:57 PM
Unknown Object (File)
Fri, Jun 21, 5:49 PM
Unknown Object (File)
Jun 1 2024, 3:31 AM
Unknown Object (File)
Jun 1 2024, 3:31 AM
Unknown Object (File)
Jun 1 2024, 3:31 AM

Details

Summary

Relevant Linear issue here. More details in inline comments below.

Test Plan

ShellCheck, close reading.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

services/scripts/test_service.sh
6 ↗(On Diff #14204)

Not that it really matters in this particular instance since the command ends right after the variable is set, but a here string can be used here instead of the echo + pipe. In my opinion it looks cleaner and if the script did go further, it could run into some issues using echo.

18–19 ↗(On Diff #14204)

double quote to prevent globbing/word-splitting

ashoat requested changes to this revision.Jul 5 2022, 12:20 PM
ashoat added inline comments.
services/scripts/test_service.sh
8 ↗(On Diff #14204)

Should we address this single-bracket usage? Or is it addressed somewhere else?

This revision now requires changes to proceed.Jul 5 2022, 12:20 PM
services/scripts/test_service.sh
8 ↗(On Diff #14204)

Great catch! I just put up D4453 addressing it since it is the final single-bracket usage in a shell script in the codebase.

This revision is now accepted and ready to land.Jul 5 2022, 1:22 PM
This revision was automatically updated to reflect the committed changes.