app: Rename internals → ex #683
Conversation
cgwalters
force-pushed
the
kids-ex-is-for-experimental
branch
from
c6fa945 to
c0d7b21
Compare
|
Rebased on #684 since we need that to pass the CI now. (I broke this with http://pkgs.fedoraproject.org/cgit/rpms/ostree.git/commit/?id=ca89bf28a60ad8a618b58ea90c03ba332c175124 which I'd kind of intended only to do in rawhide but ended up everywhere after a git merge) |
src/app/rpmostree-builtin-ex.c
Outdated
| RpmOstreeCommand *subcommand; | ||
| const char *subcommand_name = NULL; | ||
| g_autofree char *prgname = NULL; | ||
| int exit_status = EXIT_SUCCESS; | ||
|
|
||
| subcommand_name = rpmostree_subcommand_parse (&argc, argv); | ||
|
|
||
| subcommand = internals_subcommands; | ||
| if (!rpmostree_option_context_parse (context, NULL, |
There was a problem hiding this comment.
This should be done inside the if (!subcommand->name) branch below, right? Otherwise, e.g. rpm-ostree ex unpack --help will print the help message for rpm-ostree ex rather than the unpack subcommand. This is done properly I think in the compose built-in. Maybe we can unify subcommand handling as well? (There are some other inconsistencies right now between compose and ex. E.g. rpm-ostree compose vs rpm-ostree ex return different error messages.).
There was a problem hiding this comment.
Otherwise, e.g. rpm-ostree ex unpack --help will print the help message for rpm-ostree ex rather than the unpack subcommand.
Looks like this is also an issue with the container subcommands.
I think this ends up being cleaner - the properties of a command are now consistently represented as flags. Prep work for coreos#682
The goal is to consolidate our "experimental" functionality under one subcommand. This makes it easier to determine when things "graduate" to permanent-stability status under the main command line. Closes: coreos#682
Now all of our "experimental" bits are under one roof, so we get consistent handling for them.
cgwalters
force-pushed
the
kids-ex-is-for-experimental
branch
from
c0d7b21 to
b6dcc09
Compare
|
The problem is that we needed to handle the |
| g_assert (invocation); | ||
|
|
||
| g_printerr ("%snotice%s: %s is an experimental command and subject to change.\n", | ||
| bold_prefix, bold_suffix, invocation->command->name); |
There was a problem hiding this comment.
I think invocation->command->name will always be ex here. What we actually want is command_name below, right? Alternatively, the commands under ex also need to be flagged as EXPERIMENTAL but not ex itself? Hmm, kinda messy.
the properties of a command are now consistently represented as flags
That makes sense, though for EXPERIMENTAL, since only ex will use it, it might be cleaner to drop that flag and print the notice in rpmostree_builtin_ex()?
There was a problem hiding this comment.
I guess...though I dunno that it's really valuable to polish this extensively - time I spend tweaking the fine details of the code here is time not spent on actually writing livefs or tests etc.
|
OK, we can polish later. |
Closes: #683 Approved by: jlebon
Now all of our "experimental" bits are under one roof, so we get consistent handling for them. Closes: #683 Approved by: jlebon
|
☀️ Test successful - status-atomicjenkins |
This is a follow-up to coreos#683. All the experimental commands will be under `rpm-ostree ex`, so we remove the flag and instead print the notice before dispatching the subcommand, where we can print the correct name.
The goal is to consolidate our "experimental" functionality under one
subcommand. This makes it easier to determine when things "graduate"
to permanent-stability status under the main command line.
Closes: #682