Repository navigation
[cli] Introduce images command to replace find - #5274
Merged
Merged
Conversation
Laefy
force-pushed
the
cnoel/multipass-images
branch
3 times, most recently
from
September 18, 2026 13:45
0c9c078 to
89f5950
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5274 +/- ##
==========================================
+ Coverage 72.02% 72.03% +0.02%
==========================================
Files 347 349 +2
Lines 19081 19093 +12
==========================================
+ Hits 13741 13752 +11
- Misses 5340 5341 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Laefy
force-pushed
the
cnoel/multipass-images
branch
from
September 18, 2026 14:34
89f5950 to
63042aa
Compare
ricab
reviewed
Sep 22, 2026
ricab
left a comment
ricab
left a comment
Collaborator
There was a problem hiding this comment.
Hi @Laefy, I just took this for a quick spin as you requested. I had a brief look at the changes too. Here is a preliminary review with my first feedback.
Well done 👍
- Overall, the surface makes sense (I did not look closely at the code though).
- I am happy to see the PR encompassing all that the renaming entails (CLI, GUI, RPC, daemon, tests, docs, bash completion).
- I like how you preserve the deprecated command without duplicating code.
- You use the right output stream (cerr).
Suggestions
A couple of suggestions for the future (but not necessary to go back right now):
- Prefer
git clang-formatfor formatting.- That formats only your diff and avoids reformatting code that you did not otherwise touch, helping reviewers focus on what matters and reducing the chance of conflicts in other PRs.
- You can have a hook to reformat before you commit, for example. I have one that fails the commit if there are formatting changes. Let me know if you want it.
- I encourage you to split work in atomic but detailed sequences of commits, representing refined logical steps to get to the solution.
- In this situation, I understand there is a tension, because renaming the command in some places but not others would leave things in a half-done state, where compilation and tests could break and docs would be out of sync.
- But we have the pull request level to ensure consistency. When the changes are small, avoiding broken compilation and tests is a good idea (which helps with things like bisect). However, when a change reaches hundreds of LoC in many different files, justification for inner splits mount, preferably with a note in commit messages.
- Again, the PR needs to ensure that the overall changes are coherent, but it is a good idea preserve a record of the path to get there. See more in our guidelines on the matter
I am very happy you to see you tackle this task. Given how the code is structured, it is less obvious than it may appear at first glance, as I suspect you are already finding 😉 But it is something that will improve users' first contact has long deserved attention.
Keep it up!
Laefy
force-pushed
the
cnoel/multipass-images
branch
7 times, most recently
from
September 29, 2026 12:40
0af9380 to
ab150b1
Compare
Laefy
marked this pull request as ready for review
September 29, 2026 12:40
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical and moderate issues affect warning behavior, JSON output, testing, and deprecated-command completion.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
Introduces images as the official command and retains find as a deprecated alias, updating RPCs, clients, tests, documentation, and completion.
Changes:
- Renames image RPCs, messages, handlers, and formatters.
- Adds command-name detection and deprecation warnings.
- Updates GUI bindings, tests, documentation, and Bash completion.
Review findings:
completions/bash/multipass— moderate, 2 votes: deprecatedfindstill receives option completions.include/multipass/user_messages/deprecation_warning.h— moderate, 4 votes: warning format no longer matches the repeated-warning suppression regex.src/client/cli/cmd/images.cpp— critical, 2 votes: CLI tests do not verify alias warning behavior.tests/cli/cli_images_test.py— critical, 1 vote: merged stderr causes JSON parsing failures for deprecated-alias cases.tests/cli/cli_images_test.py— nit, 3 votes: fixture comment typo.
| File | Reviewed change |
|---|---|
tests/unit/test_output_formatter.cpp |
Updates formatter tests for image replies. |
tests/unit/test_format_utils.cpp |
Updates image reply fixtures. |
tests/unit/test_daemon.cpp |
Tests the renamed images RPC. |
tests/unit/test_daemon_images.cpp |
Renames daemon image tests. |
tests/unit/test_cli_client.cpp |
Tests both command names. |
tests/unit/mock_daemon.h |
Updates daemon mocks. |
tests/unit/mock_client_rpc.h |
Updates RPC stub mocks. |
tests/unit/CMakeLists.txt |
Registers renamed tests. |
tests/cli/README.md |
Updates CLI examples. |
tests/cli/pyproject.toml |
Renames the test marker. |
tests/cli/controller/multipassd_governor.py |
Uses images for readiness checks. |
tests/cli/cli_images_test.py |
Tests both command names. |
tests/cli/cli_help_test.py |
Updates help expectations. |
src/user_messages/CMakeLists.txt |
Adds the user-messages interface library. |
src/rpc/multipass.proto |
Renames image RPC definitions. |
src/platform/backends/shared/base_vm_image_vault.h |
Updates command references. |
src/daemon/daemon.h |
Renames the daemon method. |
src/daemon/daemon.cpp |
Implements the images RPC and warning helper. |
src/daemon/daemon_rpc.h |
Renames RPC declarations. |
src/daemon/daemon_rpc.cpp |
Renames RPC dispatch. |
src/daemon/CMakeLists.txt |
Links user messages. |
src/CMakeLists.txt |
Registers the new library. |
src/client/gui/lib/grpc_client.dart |
Updates GUI RPC bindings. |
src/client/gui/lib/catalogue/catalogue.dart |
Uses the images client method. |
src/client/cli/formatter/yaml_formatter.cpp |
Updates YAML formatting. |
src/client/cli/formatter/table_formatter.cpp |
Updates table formatting. |
src/client/cli/formatter/json_formatter.cpp |
Updates JSON formatting. |
src/client/cli/formatter/csv_formatter.cpp |
Updates CSV formatting. |
src/client/cli/cmd/images.h |
Defines the renamed command. |
src/client/cli/cmd/images.cpp |
Implements images and alias handling. |
src/client/cli/cmd/CMakeLists.txt |
Registers command sources and dependencies. |
src/client/cli/client.cpp |
Registers the images command. |
src/client/cli/argparser.cpp |
Adds command-name retrieval. |
README.md |
Updates command documentation. |
include/multipass/user_messages/driver_deprecation_warning.h |
Extracts driver warning construction. |
include/multipass/user_messages/deprecation_warning.h |
Adds shared deprecation formatting. |
include/multipass/cli/yaml_formatter.h |
Updates formatter API. |
include/multipass/cli/table_formatter.h |
Updates formatter API. |
include/multipass/cli/json_formatter.h |
Updates formatter API. |
include/multipass/cli/formatter.h |
Renames the formatter interface. |
include/multipass/cli/csv_formatter.h |
Updates formatter API. |
include/multipass/cli/argparser.h |
Declares commandName(). |
docs/tutorial/getting-started.md |
Updates tutorial commands. |
docs/reference/gui-client.md |
Updates GUI documentation. |
docs/reference/command-line-interface/launch.md |
Links to images. |
docs/reference/command-line-interface/images.md |
Documents the renamed command. |
docs/index.md |
Updates introductory examples. |
docs/how-to-guides/troubleshoot/troubleshoot-launch-start-issues.md |
Updates troubleshooting commands. |
docs/how-to-guides/manage-instances/create-an-instance.md |
Updates instance guidance. |
docs/explanation/image.md |
Updates image documentation. |
completions/bash/multipass |
Adds images completion support. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Laefy
force-pushed
the
cnoel/multipass-images
branch
from
September 29, 2026 14:14
ab150b1 to
4ce7d9a
Compare
Laefy
added this pull request to stack #5328
September 29, 2026 14:26
Laefy
force-pushed
the
cnoel/multipass-images
branch
from
September 30, 2026 08:51
4ce7d9a to
003644a
Compare
ricab
requested changes
Oct 1, 2026
Collaborator
|
It looks like GitHub lost one of my comments after it showed it to me twice and I deleted one of the duplicates. Adding it now: It would probably be a good idea to introduce a redirect. I used this snippet in the umount PR and I am waiting for @geoffreynyaga's confirmation that it is the right approach. |
Laefy
force-pushed
the
cnoel/multipass-images
branch
from
October 2, 2026 14:36
003644a to
b9e7623
Compare
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Three unresolved findings include one critical compile issue and two moderate behavior/documentation issues.
Review effort: Lite
Findings: 4
Open (4)
images.cpp, but the implementation changes shown are in…images.cpp, but the implementation changes shown are in…src/client/cli/cmd/find.h, but the client now…src/client/cli/cmd/find.h, but the client now…
Laefy
force-pushed
the
cnoel/multipass-images
branch
from
October 7, 2026 09:06
e707629 to
9926c7e
Compare
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Address the deprecated-alias completion, warning timing, and RPC compatibility issues.
Review effort: Lite
Findings: 4
Open (4)
images.cpp, but the implementation changes shown are in…images.cpp, but the implementation changes shown are in…src/client/cli/cmd/find.h, but the client now…src/client/cli/cmd/find.h, but the client now…
ricab
previously approved these changes
Oct 8, 2026
ricab
left a comment
ricab
left a comment
Collaborator
There was a problem hiding this comment.
Thanks Céline! This all looks good to me now, but I would like to invoke @xmkg secondary review on the CLI tests part. In particular, I don't understand what those with multipass(...).json() as output statements were doing in the first place and I would like confirmation that they are safe to remove, as well as advise on whether other such statements should also be removed after this is merged.
Other than that, I have only a tiny request inline. I will pre-approve, with the understanding that this waits for @xmkg's green light before merging.
Laefy
force-pushed
the
cnoel/multipass-images
branch
from
October 9, 2026 07:27
9926c7e to
dd5a6dc
Compare
- Rename all code associated to the `find` command appropriately. - Update documentation and tests to reflect the change from `find` to `images`. - Add `ArgParser::commandName` that returns the name with which the command was invoked. - Update the command so that `images` is the official name, and `find` is an alias for it. In case `find` is used, display a deprecation warning. - Update the autocomplete scripts. - Update the CLI and unit tests so that they check invocations on both `images` and `find`.
Add a new library to contain all user-facing messages. This has the following benefits: - it will be easier to have uniformly styled messages accross the application, - when localization feature will be introduced, collecting the call sites to adapt will be a lot easier. This commit only introduces the library itself (INTERFACE until we have .cpp to put in it) and move the driver deprecation warning into it.
Laefy
force-pushed
the
cnoel/multipass-images
branch
from
October 9, 2026 07:28
dd5a6dc to
e51888c
Compare
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Two unresolved moderate issues affect alias warning behavior and RPC compatibility.
4 open findings
images.cpp, but the implementation changes shown are in…images.cpp, but the implementation changes shown are in…src/client/cli/cmd/find.h, but the client now…src/client/cli/cmd/find.h, but the client now…
🧠 Review effort: Lite
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Fix the incompatible QString and std::string comparison in src/client/cli/cmd/images.cpp.
4 open findings
images.cpp, but the implementation changes shown are in…images.cpp, but the implementation changes shown are in…src/client/cli/cmd/find.h, but the client now…src/client/cli/cmd/find.h, but the client now…
🧠 Review effort: Lite
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
findcommand appropriately.findtoimages.ArgParser::commandNamethat returns the name with which thecommand was invoked.
imagesis the official name, andfindis an alias for it. In case
findis used, display a deprecationwarning.
imagesandfind.in order to reuse the same patterns in the
finddeprecation warning.Related Issue(s)
Part of MULTI-2696
Testing
images, but not forfindanymore.find, and not when usingimages.Checklist