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

Feat/CCT-467: Add explanatory info about srcs of overrides in repo-override cmd #3474

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

grunwmar
Copy link

The requrements are to put statements containing additional info about subscription-manager repo-override --list and about subscription-manager repo-override --remove to respective parts of man pages.

Additional info for --list:

Source of the listed overrides is a union of the override specified at the system itself and all the overrides set in the environment(s) system belongs in.

Additional info for --remove:

Source of the override that is attempted to be removed may be one of the environment(s) the system belongs in. It can only be removed by either: removing the override from the environment it originates from or removing the system from the environment the override
originates from.

CARD CCT-467

Copy link
Member

@nikosmoum nikosmoum left a comment

Choose a reason for hiding this comment

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

Just a couple suggestions/enhancements

@@ -604,7 +604,8 @@ option
.B --remove=NAME
Removes a named override from the repositories specified with the
.B --repo
option
option.
Source of the override that is attempted to be removed may be one of the environment(s) the system belongs in. It can only be removed by either: removing the override from the environment it originates from or removing the system from the environment the override originates from.
Copy link
Member

Choose a reason for hiding this comment

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

The first sentence could be a bit more straightforward, like: The override will not be removed if its source is one of the environments the system belongs in. It can only...

@@ -604,7 +604,8 @@ option
.B --remove=NAME
Removes a named override from the repositories specified with the
.B --repo
option
option.
Source of the override that is attempted to be removed may be one of the environment(s) the system belongs in. It can only be removed by either: removing the override from the environment it originates from or removing the system from the environment the override originates from.

.TP
.B --remove-all
Copy link
Member

Choose a reason for hiding this comment

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

Let's also extend the info on this command, by adding: ... --repo option. Overrides inherited from environments the system belongs in will not be removed. They can only be removed by either: removing them from the environment they originate from or removing the system from those environments.

@@ -616,7 +617,8 @@ option
.B --list
Lists all overrides from repositories specified with the
.B --repo
option
option. Source of the listed overrides is a union of the override specified at the system itself and all the overrides set in the environment(s) system belongs in.
Copy link
Member

Choose a reason for hiding this comment

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

minor correction: there is a missing the between the words environment(s) and system

…epo-overrides

command

The requrements are to put statements containing additional info about `subscription-manager repo-override --list`
and about `subscription-manager repo-override --remove` to respective parts of man pages.

CARD CCT-467
Copy link
Contributor

@zpetrace zpetrace left a comment

Choose a reason for hiding this comment

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

Hi, the changes looks good to me from the QE side, more info is in the verification comment in the corresponding CCT card:)

option

option. Overrides inherited from environments the system belongs in will not be removed. They can only be removed by either: removing them from the environment they originate from or removing the system from those environments.
The source will not be removed if its source is one of the environment(s) the system belongs in. It can only be removed by either: removing the override from the environment it originates from or removing the system from the environment the override originates from.
Copy link
Member

Choose a reason for hiding this comment

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

I think you added the text I proposed for the --remove-all command into the --remove command. If you check my other comment, it was done against line 610 (where the --remove-all command description is)

Copy link
Contributor

@zpetrace zpetrace left a comment

Choose a reason for hiding this comment

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

Ahh yes, Niko is right, I didn't notice as well before, the text is correct but should be moved to --remove-all. Thanks for pointing that out @nikosmoum :)

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.

3 participants