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

tag-delta: Allow listing only downgrades or only upgrades #19

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

compi-migui
Copy link
Contributor

@compi-migui compi-migui commented Jun 16, 2023

Only really needed downgrades_only, but threw in the converse while I was at it.

Comment on lines +89 to +100
for c in sorted(delta_info['downgrades'].keys()):
lnevr = delta_info['downgrades'][c]['old']
rnevr = delta_info['downgrades'][c]['new']

levr = evr_from_nevr(lnevr)
revr = evr_from_nevr(rnevr)

if sys.stdout.isatty():
f = '%' + fw + 's %s%' + fw + 's %s%' + fw + 's'
print(f % (c, decor.HILIGHT, levr, decor.NORMAL, revr))
else:
print(f'{c},{levr},{revr},')
if delta_info['downgrades'][c]['rebase']:
print(f'{c},{levr},{revr},rebase')
else:
print(f'{c},{levr},{revr},')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diff is having a hard time with this. The block is indented one place right to be under the new if show_downgrades: conditional, there's no other changes.

Comment on lines +106 to +117
for c in sorted(delta_info['upgrades'].keys()):
lnevr = delta_info['upgrades'][c]['old']
rnevr = delta_info['upgrades'][c]['new']

levr = evr_from_nevr(lnevr)
revr = evr_from_nevr(rnevr)
levr = evr_from_nevr(lnevr)
revr = evr_from_nevr(rnevr)

if sys.stdout.isatty():
f = '%' + fw + 's %' + fw + 's %s%' + fw + 's%s'
print(f % (c, levr, decor.HILIGHT, revr, decor.NORMAL))
else:
if delta_info['upgrades'][c]['rebase']:
print(f'{c},{levr},{revr},rebase')
if sys.stdout.isatty():
f = '%' + fw + 's %' + fw + 's %s%' + fw + 's%s'
print(f % (c, levr, decor.HILIGHT, revr, decor.NORMAL))
else:
print(f'{c},{levr},{revr},')
if delta_info['upgrades'][c]['rebase']:
print(f'{c},{levr},{revr},rebase')
else:
print(f'{c},{levr},{revr},')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above:
The diff is having a hard time with this. The block is indented one place right to be under the new if show_upgrades: conditional, there's no other changes.

Copy link

@jpichon jpichon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I'm wondering if it'd make sense to keep to the same terminology as --skip-same? So you'd have --skip-downgrades and --skip-upgrades as well, to avoid having to think in reverse depending on the flag. Then if for some reason, someone someday wants any combination of two options, they could also get it...

fuzzball81
fuzzball81 previously approved these changes Jul 5, 2023
@compi-migui
Copy link
Contributor Author

LGTM! I'm wondering if it'd make sense to keep to the same terminology as --skip-same? So you'd have --skip-downgrades and --skip-upgrades as well, to avoid having to think in reverse depending on the flag. Then if for some reason, someone someday wants any combination of two options, they could also get it...

hmm. For my use case this made much more sense, as I very often find myself wanting to list downgrades or upgrades and basically never need to see the intersection of the two tags. If anything (for how I use tag-delta) I'd get rid of --skip-same, making that behavior the default and have a --show-same option to include the intersection.

But it's totally fair that we'd want to keep the new options consistent with the existing one. I'll send up a new revision doing that.

Separately to that, if I have the time, I may propose reworking all the options in a way that makes them easier to use while keeping them internally consistent and hopefully also backwards-compatible. Need to think about that some more.

The new options can be combined to, e.g., only list downgrades by
passing both --skip-same and --skip-upgrades.
@jguiditta jguiditta enabled auto-merge (rebase) July 25, 2023 18:20
Copy link
Member

@jguiditta jguiditta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@jguiditta jguiditta merged commit a430a57 into release-depot:master Jul 25, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants