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.

Sign up for GitHub

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

Jump to bottom

Refs #33620 - limit target version upto N+1 #602

Merged
merged 2 commits into from
Apr 6, 2022
Merged

Refs #33620 - limit target version upto N+1 #602

merged 2 commits into from
Apr 6, 2022

Conversation

upadhyeammit
Copy link
Contributor

This makes,

  1. The target-version optional, actually I think if we agree on this then we don't require target-version at all.
  2. Validates if target version is only N+1 where N is the current installed version.

@theforeman-bot
Copy link
Member

Issues: #33620

@upadhyeammit upadhyeammit requested a review from ehelms April 1, 2022 11:39
@ehelms
Copy link
Member

ehelms commented Apr 1, 2022

Looks good to me, I'd like to see what @evgeni thinks to to vet it across another.

@evgeni
Copy link
Member

evgeni commented Apr 1, 2022

Yeah, I would drop the "target version" completely in this case. What's the point of a parameter that can only ever have one value.

@upadhyeammit
Copy link
Contributor Author

Waiting on #604 to merge to rebase this pr

@evgeni
Copy link
Member

evgeni commented Apr 5, 2022

LGTM

@upadhyeammit upadhyeammit merged commit 8f1ee8d into theforeman:master Apr 6, 2022
@upadhyeammit upadhyeammit deleted the 33620-limit-version branch April 6, 2022 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ehelms ehelms ehelms approved these changes

Assignees
No one assigned
Labels
Needs testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@upadhyeammit @theforeman-bot @ehelms @evgeni