iterative
Introduce subcommands in a backwards-compatible way
This pull request renames all the commands (loosely as proposed) in a backwards-compatible way.
Functionality remains unchanged, and the only difference is that cml <balderdash> can also be called by using cml <noun> <verb> as suggested.
- [x] depends on #1026 (to avoid rebase hell)
- [ ] Doesn't quite close https://github.com/iterative/cml/issues/762
- [ ] opens a
cml.devdocs issue (including for #1026)
🔔 @iterative/cml, ready for a first round of feedback?
For now, the only breaking change (as per our last meeting) is that calling cml runner with no arguments prints the help message instead of launching a local runner.
The rest of commands are backwards compatible ad nauseam and behave exactly as they did in the past, thanks to a fair amount of yargs surgery. 🤢
Reading the pull request changes is rather cumbersome. Given that tests haven't changed and are still passing,[^1] I would recommend to npm install --global github:iterative/cml#command-naming-consistency and explore the new commands as users would do.
[^1]: For what they're worth, of course.
looking at the code it looks like you can only call it as cml runner start and there is no legacy mode cml runner is that incorrect?
That's incorrect, or at least was incorrect the last time I checked. 🤔 As stated on https://github.com/iterative/cml/pull/1073#issuecomment-1163383184, the only breaking change is that calling cml runner without any options prints the help message.
@iterative/cml, any objections to merging this? Rebasing forever and ever is a bit complicated and Sisyphus was the only who laughed.
dashboard/integrations like tensorboard are
connected orpublished to... A background daemon monitors & streams(again, NOTstarted. Functionality is extremely similar tocml report watch --publish... so let's not call itstart?)
To my mind, integrations like Tensorboard use daemons/agents/clients/servers/... which have to be cough started in order to communicate information to the user. Maybe using the verb start everywhere is too reductionist, but having different verbs for every command whose function is equivalent introduces an unneccessary strain in users.
Start TensorBoard through the command line or within a notebook experience. (Excerpt from Tensorboard's Get Started Guide)
my tuppence: starting daemons is an implementation detail users need not care about.
regarding the "overly generic" versus "accurate but not well-known" naming clashes: I'd pick a third and fourth option: "consistent" (stick to our existing docs) and "well-known" (avoid engineering/implementation-specific jargon terms, instead target matching the end-user's intuition)
"consistent" (stick to our existing docs)
That's a bit of a double-edged sword: the whole point of this pull request is renaming commands for consistency and, as such, will invariably require the documentation to match the changes.
"well-known" (avoid engineering/implementation-specific jargon terms, instead target matching the end-user's intuition)
Up. Goer. Five.
Engineering terms are well documented and allegedly close to our target audience's mindset.
On the other hand, “matching the end-user's intuition” is not a trivial process, and, lacking extensive research on the topic, easily biased by our individual preferences.
Do we pretend to have on file per action for command entry point having the logic in
/src/cml.js? 😰
Yes, unless there is a better alternative, of course.
There is a lot of repeated boilerplate.
If it were easy to generalize, it would be in a single file. Unfortunately, there are some differences (commands with the same name as the legacy counterpart) that require custom tweaks. Suggestions are welcome.
This is working
cml runner runner --labels cml --single --repo https://github.com/DavidGOrtega/fashion_mnist --token XXXX
Rebase mistake, fixed with https://github.com/iterative/cml/pull/1073/commits/36c2439fbebce13cba47d7b2d37e71b94cdd9a66 🙈
Would make sense to move legacy.js within the legacy folder?
Not much, because
yargswould try to parse it as a subcommand, and- breaks the aesthetics of having matching
.jsfiles and the pertaining directories[^1]
[^1]: silly reason, originated in the OCD I don't have ™️
2. breaks the aesthetics of having matching
.jsfiles and the pertaining directories1
Its currently breaking my own OCD aesthetics
🔔 @iterative/cml, I would recommend one last review; not to the code, but to the user experience. E.g. run cml --help and navigate the help menus for every subcommand.
(To be continued, please check resolved comments just in case some resolution was too heavy-handed; I had to mark them somehow to see clearly in that mess)
Possible to change the options ordering so that the "Global Options" appear last?
Possible to change the options ordering so that the "Global Options" appear last?
Still haven't figured out how to do that; assuming it's not possible unless proven otherwise. 🤔
Next steps
- @iterative/cml, please make sure that you're happy with the result
- @iterative/scapegoat, please open follow-up issues for the items listed in the pull request body