cli: Add a --check flag for update
#105
Conversation
|
Hi @jbtrystram. Thanks for your PR. I'm waiting for a containers member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
| } | ||
| PrepareResult::Ready(p) => { | ||
| println!( | ||
| "New manifest available for {}. Digest {}", |
There was a problem hiding this comment.
One less than ideal thing here is that it leaves open the problem of how programmatic access would work - if I'm scripting bootc, I'd have to parse this text designed for human readability.
We already have a --touch-if-changed CLI option which is designed for the non-check case, so let's honor it with --check too.
| return Ok(()); | ||
| } | ||
| } else { |
There was a problem hiding this comment.
Since we have a return above I don't think we need to indent everything here, right? (Though if you follow my above suggestion, the touch_if_changed handling would be outside the else)
There was a problem hiding this comment.
I updated the PR with a change in the return statement and moved the touch_if_changed handling outside of the else. Now it will carry on and honor the option.
However that keeps the indentation.
|
/ok-to-test |
|
Two minor nits on the commit message
|
|
BTW, this will conflict with #106 - I forgot you'd been looking at this issue. How these two intersect is actually very interesting because semantically that's about adding a declarative flow...so to properly model things here we may need to split out a "discovered but not downloaded" state internally. |
This simply pull the manifest of the image to see if any update and available. It could be named `--dry-run` but I wanted to be consistent with `rpm-ostree` Fixes bootc-dev#3 Signed-off-by: jbtrystram <[email protected]>
jbtrystram
force-pushed
the
update-dry-run
branch
from
74c29d6 to
a022d80
Compare
I amended the commit message fixing that :) |
which PR do you want to merge first ? I'm fine waiting for your changes then rebasing this |
cgwalters
changed the title
--check flag for update.--check flag for update
Mmmm...my inclination here is to merge your work first because I want to prioritize new contributors. So let's do that. Thanks for the PR! |
This simply pull the manifest of the iamge to see if any update and available. It could be named
--dry-runbut I wanted to be consistent withrpm-ostree.Fixes #3