Small reorganization in Y2Issues #1178
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
imobachgs
approved these changes
Jun 23, 2021
ancorgs
commented
Jun 25, 2021
| @@ -37,45 +38,101 @@ class Reporter | |||
| def initialize(issues, report_settings: Yast::Report.Export) | |||
| textdomain "base" | |||
| @presenter = Presenter.new(issues) | |||
| @level = issues.fatal? ? :error : :warn | |||
| @level = issues.error? ? :error : :warn | |||
| @log, @show, @timeout = find_settings(report_settings, @level) | |||
There was a problem hiding this comment.
@imobachgs I was thinking about this. We are deciding which AutoYaST settings to use (<warnings> vs <errors>) based on the severity. But I was wondering whether it would make sense to use <yesno_messages> in the cases in which we ask the user whether to continue or not (when ask is used for the corresponding severity).
There was a problem hiding this comment.
And talking about honoring the AutoYaST settings, what's the rationale to ignore the timeout and enforce it to zero in for the "error" level?
|
✔️ Public Jenkins job #315 successfully finished |
|
✔️ Internal Jenkins job #153 successfully finished |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related pull requests
Problem
During the partial re-implementation of yast2-users, we found a couple of things to be improved in Y2Issues.
:warnand:fatal. But other parts of YaST use slightly different conventions for similar concepts[1]:Reportthere are functions line,Message,WarningandError.<report>section of the AutoYaST profile, it's possible to define the behavior for the following categories: errors, warnings, messages, yesno_messages.Y2Issues.reportwas too focused in a single use case - showing errors found while processing the AutoYaST profile and offering the user the opportunity to abort installation.Solution
:fatalto:error.Y2Issue.reportwith two new arguments that allow to specify what to do depending on the severity of the issues.Testing
Notes
[1] The installation proposals follow a different convention, but I don't think it's expected to use Y2Issues in that context. Just for completeness, let's list here the warning levels accepted for a proposal:
:notice:warning(default):error:blocker:fatalA
:blockerwill prevent the user from continuing the installation. If any proposal contains a blocker warning, the Accept button in the proposal dialog will be disabled - the user needs to fix that blocker before continuing.:fatalis like:blockerbut also stops building the proposal.An
:errordoes not prevent continuing of the installation, but shows a popup that an user has to confirm to continue with the installation.