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

Improve Driver Disks handling as Supplemental Packs #73

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ydirson
Copy link
Contributor

@ydirson ydirson commented Oct 18, 2023

This PR improves the final "Supplemental Packs" dialog when the user made use of Driver Disk(s), and also fixes the handling of Driver Disks when the supplemental-packs feature is disabled (re-enabling the dialog, with the more targeted message).

This is a followup to #28.

tui/installer/screens.py Outdated Show resolved Hide resolved
tui/installer/screens.py Outdated Show resolved Hide resolved
In interactive mode, when Driver Disks are used as local media, they
must *also* be installed as Supplemental Packs after installation.
This change remembers the use of Driver Disks and makes sure the
question is asked, even when the supplemental-packs feature is
disabled.

Signed-off-by: Yann Dirson <yann.dirson@vates.fr>
It may not be obvious to users that local-media Driver Disks must be
inserted twice during installation.  This change uses the memorization
of usage of Driver Disks from previous commit, and uses it to explain
users they likely wants to insert their Driver Disk again.

Signed-off-by: Yann Dirson <yann.dirson@vates.fr>
@stormi
Copy link
Contributor

stormi commented Dec 6, 2023

Ping to reviewers 👋

@freddy77
Copy link
Collaborator

freddy77 commented Dec 6, 2023

@stormi before the ping you should make sure all discussions are resolved. By the way, checked they were resolved. Fine for me

@freddy77 freddy77 self-requested a review December 6, 2023 15:07
@stormi
Copy link
Contributor

stormi commented Dec 6, 2023

I did check they were resolved, at least to my understanding :). I rarely ping despite many pending reviews, by the way.

freddy77
freddy77 previously approved these changes Dec 6, 2023
@stormi
Copy link
Contributor

stormi commented Dec 6, 2023

If you meant the conversations were still open from github's point of view, it's not always obvious in review processes whose responsibility it is to mark them as resolved. The committer, or the reviewer? I usually prefer when it's the reviewer, this way they have a way to have a last look to what they asked and how it was answered, before marking the conversation as resolved. Others may prefer that they are marked as resolved by the committer.

@freddy77
Copy link
Collaborator

freddy77 commented Dec 6, 2023

If you meant the conversations were still open from github's point of view, it's not always obvious in review processes whose responsibility it is to mark them as resolved. The committer, or the reviewer? I usually prefer when it's the reviewer, this way they have a way to have a last look to what they asked and how it was answered, before marking the conversation as resolved. Others may prefer that they are marked as resolved by the committer.

It sounds reasonable. Personally on my changes if I changed as requested/suggested I put a done comment and mark as resolved, otherwise I put a comment for the specific reviewer (why I didn't exactly follow the request or why I don't think it should be done, for instance because it will be done separately).

@ydirson
Copy link
Contributor Author

ydirson commented Jun 20, 2024

any news on this one?

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