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

Add support for non-critical installation errors #3147

Merged
merged 1 commit into from
Feb 8, 2021

Conversation

poncovka
Copy link
Contributor

@poncovka poncovka commented Feb 4, 2021

The OSCAP add-on needs to be able to raise a non-critical installation error.
In that case, the users should be allowed to continue with the installation
if they want to.

@poncovka poncovka added master Please, use the `f39` label instead. manual testing required This issue can't be merged without manual testing labels Feb 4, 2021
Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Looks good to me so far.

Comment on lines 212 to 214
message = _("The following error occurred during the installation:")
message += "\n\n" + str(exn) + "\n\n"
message = _("Would you like to ignore this and continue with installation?")
Copy link
Contributor

Choose a reason for hiding this comment

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

As a translator, I would definitely prefer one string to many small pieces. If it's not inconvenient, please make it one string with format or concatenation, as in the handlers above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

The OSCAP add-on needs to be able to raise a non-critical installation error.
In that case, the users should be allowed to continue with the installation
if they want to.
@poncovka poncovka force-pushed the master-non_critical_errors branch from d214acb to f1ce918 Compare February 4, 2021 19:37
@poncovka
Copy link
Contributor Author

poncovka commented Feb 4, 2021

uups

@poncovka poncovka removed the manual testing required This issue can't be merged without manual testing label Feb 4, 2021
@poncovka poncovka marked this pull request as ready for review February 4, 2021 19:42
@poncovka
Copy link
Contributor Author

poncovka commented Feb 5, 2021

/kickstart-test --testtype smoke

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master Please, use the `f39` label instead.
Development

Successfully merging this pull request may close these issues.

2 participants