tinypilot
tinypilot copied to clipboard
Consolidate naming and responsibility of dev scripts
The build-python
and build-javascript
dev scripts technically don’t build anything, but they rather execute tests and static analysis procedures. Therefore, the prefix build-
might be a bit surprising. To me, “build” would indicate that something is compiled or assembled. I’d probably find test-
more intuitive here.
We also have several separate “check”-prefixed bash scripts that do static analysis. So it might not be totally clear where to put things. E.g., Python style checking is performed via build-python
, whereas JavaScript style checking is performed via check-style
and not via build-javascript
. As we are about to add tests for bash scripts, we will be having the same duality with check-bash
and build-bash
.
Seeing that we have 20 dev scripts by now, I suggest we take a step back and review their structure altogether, and try to find a more consistent naming and responsibility scheme.
Yeah, the naming is not super well-thought-out. It's mainly that the scripts match conventions I use in previous projects, and I'm used to the naming at this point, so I don't re-evaluate.
I agree that it's confusing because there's not a clear pattern on when to use build
vs check
vs lint
.
I think we should come up with a small, simple set of rules that we feel comfortable following and adapting our scripts to match.
I’ve dabbled a bit with the structure of the scripts, and to me, the following changes would seem sensible:
(Note, the branch doesn’t reflect all of these suggestions yet, it’s more like a rough initial exploration.)
- [x] Rename all “check” scripts to have a
check-
prefix (regardless of whether they are linting, testing, or style checking) - [x] Add missing invocations to
check-all
script (previously:build
), which doesn’t cover all scripts currently - [x] Introduce
check-debian-pkg
script that contains the inlined bash from the CircleCI YAML - [x] Move the
enable-passwordless-sudo
and the 2install-
scripts to a new folderdev-scripts/remote/
, to make it clear that they are supposed to run on device (in contrast to all other scripts, which are made to run on the dev machine) - [x] Ensure that all dev scripts have a brief, initial docstring comment.
- [x] Adjust all paths of where we invoke any of the renamed scripts
@jdeanwallace what would you think about that proposed structure?
@jotaen4tinypilot - These changes look good to me.
Move the enable-passwordless-sudo and the 2 install- scripts to a new folder dev-scripts/remote/, to make it clear that they are supposed to run on device (in contrast to all other scripts, which are made to run on the dev machine)
(nit) The word "remote" makes me think of a device not in my local network. Alternative suggestion: "device"?