Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

app/dbus-helpers: Return early if no diff to print #1431

Closed
wants to merge 1 commit into from

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Jun 28, 2018

A new update might not have any package changes at all. In which case,
we shouldn't even try to print the diff, otherwise we'll fail on trying
to lookup the upgraded key. We did this check already from
rpmostree_print_cached_update, but not from print_one_deployment
(when printing the diff as part of the pending deployment), so we'd
error out on no-diff upgrades.

Let's just inline a check in the function directly instead of wrapping
every call site.

A new update might not have any package changes at all. In which case,
we shouldn't even try to print the diff, otherwise we'll fail on trying
to lookup the `upgraded` key. We did this check already from
`rpmostree_print_cached_update`, but not from `print_one_deployment`
(when printing the diff as part of the pending deployment), so we'd
error out on no-diff upgrades.

Let's just inline a check in the function directly instead of wrapping
every call site.
@cgwalters
Copy link
Member

Looks sane to me.

Oooh, the CI failures are fallout from ostreedev/ostree#1654

@jlebon
Copy link
Member Author

jlebon commented Jun 28, 2018

@rh-atomic-bot r=cgwalters 49ef988

@rh-atomic-bot
Copy link

⌛ Testing commit 49ef988 with merge 1b5ab98...

@rh-atomic-bot
Copy link

☀️ Test successful - status-atomicjenkins
Approved by: cgwalters
Pushing 1b5ab98 to master...

@jlebon jlebon deleted the pr/fix-status-noop branch April 23, 2023 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants