Add fast deployment #339
Add fast deployment #339
Conversation
| "-f", | ||
| "--fast", | ||
| action="store_true", | ||
| help="Don't build/compile on the target machines, instead synchronize the complete install directory from the local machine.", |
There was a problem hiding this comment.
I would write build/install, as we are syncing both as far as I've seen.
| @@ -99,6 +100,12 @@ def _parse_arguments(self) -> argparse.Namespace: | |||
| ) | |||
|
|
|||
| # Optional arguments | |||
| parser.add_argument( | |||
There was a problem hiding this comment.
I personally would suggest renaming or even skipping the -f shorthand, because in a lot of tools -f stands for force and breaks with general CLI tooling convention.
Also, as long as we are not using the -f flag for multiple combined speedup options, I would suggest defining a dest with a clear variable name, as with the other arguments.
E.g. sync_build_alternative or just skip_build or something of the sort.
| @@ -140,6 +147,12 @@ def _parse_arguments(self) -> argparse.Namespace: | |||
| args.build = args.only_build | |||
| args.launch = args.only_launch | |||
|
|
|||
| if args.fast: | |||
There was a problem hiding this comment.
I think it's generally fine, but now you could theroretically pass both --build and --fast and get a potentially confusing outcome for the user, as changed files on the robot are also cleaned and no build is happening.
| self._remote_workspace = remote_workspace | ||
| self._package = package | ||
| self._pre_clean = pre_clean | ||
| self._fast = fast |
There was a problem hiding this comment.
I think fast is especially wrong in a Sync class, seeing as the non fast sync includes less files and should actually be faster.
| "'--include=+ install'", | ||
| "'--include=+ install/**'", | ||
| "'--include=+ src'", | ||
| "'--include=+ src/**'", |
There was a problem hiding this comment.
Could including all src/** subfolders lead to later build/execution issues, when things are copied that are not part of our sync includes?
|
We don't want to finish this, as the feature "fast deployement" my introduce errors and we can only use this in a few situations. Current methods work sufficiently. We don't need to introduce this additional dirty complexity. |
Summary
Related issues
Checklist
colcon buildSoftwareproject board