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

Fix expert partitioner shortcut in sle15/old storage-ng stack #6397

Merged
merged 1 commit into from
Dec 13, 2018
Merged

Fix expert partitioner shortcut in sle15/old storage-ng stack #6397

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions lib/partition_setup.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,17 @@ sub create_new_partition_table {
send_key_until_needlematch "expert-partitioner-vda", 'right';

# empty disk partitions by creating new partition table
my $expert_menu_key = (is_storage_ng) ? 'alt-r' : 'alt-x'; # expert menu keys
# in sle15sp1 is called Pa{r}tition Table
my $expert_menu_key = (is_storage_ng) ? ((is_storage_ng_newui) ? 'alt-r' : 'alt-e') : 'alt-x'; # expert menu keys
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 we are getting into some wrong direction here. What would happen when in SP2 they will change ui one more time ? something like this ?

Suggested change
my $expert_menu_key = (is_storage_ng) ? ((is_storage_ng_newui) ? 'alt-r' : 'alt-e') : 'alt-x'; # expert menu keys
my $expert_menu_key = (is_storage_ng) ? ((is_storage_ng_newui) ? ((is_storage_ng_themostnewui ) ? 'alt-r' : 'alt-q' ): 'alt-e') : 'alt-x'; # expert menu keys

Copy link
Contributor

Choose a reason for hiding this comment

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

So with this future forecast direction changed how ?

Copy link
Contributor

@dzedro dzedro Dec 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Please skip meaningless sentences like direction or words that just give space for interpretation.
Do we have some info from YaST about planned change in storage(_ng) ? If not then I would not add anything that is not there.

Copy link
Member

Choose a reason for hiding this comment

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

I am just showing that proposed solution won't scale in future and we need to find some other way to handle this. If you asking for certain suggestion I would go for split everything on version basis without this "newnew" functions which confuse a lot because they "new" only limited amount of time

Copy link
Member

Choose a reason for hiding this comment

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

@asmorodskyi I agree with you, however I think this is out of scope of this PR because we already have the functional "is_storage_ng_newui". I created https://progress.opensuse.org/issues/45116 to improve that. Can you provide your thoughts there? ok for you?

Copy link
Member

Choose a reason for hiding this comment

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

Please skip meaningless sentences like direction or words that just give space for interpretation.
Do we have some info from YaST about planned change in storage(_ng) ? If not then I would not add anything that is not there.

sorry to confuse you . just to keep it clear : my "suggestion" was just anti-pattern showing how I do NOT want code look like. I don't have any insights from YaST team but I am pretty sure that sooner or later they will change this hotkey so "new" will become "old"

Copy link
Contributor

@dzedro dzedro Dec 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I did not know what you mean, if you would write it same as second comment everything would be fine.

Copy link
Contributor Author

@mloviska mloviska Dec 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Hello all! I do strongly agree with you all. This is a hotfix for QR while we are slowly starting to work on a different concept. For more information, please check poo#44054. Thus eventually we will end up with a different, more maintainable and scalable code. I would suggest to carry on with this approach for a bit more time, until we are able to deliver mentioned ticket.


if (is_storage_ng_newui) {
# partition table management has been moved from Partitions tab to Overview
send_key 'alt-o';
assert_screen 'expert-partitioner-overview';
}

wait_screen_change { send_key $expert_menu_key }; # enter Partition table menu
# enter Partition table menu
wait_screen_change { send_key $expert_menu_key };
send_key 'down';
wait_still_screen 2;
save_screenshot;
Expand All @@ -109,7 +111,7 @@ sub create_new_partition_table {
send_key((is_storage_ng) ? $cmd{next} : $cmd{ok}); # OK
send_key 'alt-p' if (is_storage_ng); # return back to Partitions tab
}
unless (is_storage_ng) {
unless (is_storage_ng_newui) {
assert_screen 'partition-create-new-table';
send_key 'alt-y';
}
Expand Down