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

Use Y2NfsClient::Widgets::NfsForm instead of NfsClient4Part #1283

Merged
merged 12 commits into from Feb 2, 2022

Conversation

ancorgs
Copy link
Contributor

@ancorgs ancorgs commented Feb 2, 2022

Problem

This pull request replaces #1282 (same content but with clear history)

Traditionally, the management of NFS shares in the YaST Partitioner has been a bit rough. The Partitioner never really provided its own UI to add/edit/delete NFS mounts. Instead, it executed an embedded version of yast2-nfs-client. That has always been problematic. For example, there were non-obvious problems when the same NFS share was mounted more than once in the current system in two different locations (admittedly a very uncommon scenario).

But the situation became much worse with the revamp of the interface done for SLE-15-SP3 (and openSUSE Leap 15.3). Since that moment, NFS management was completely inconsistent with the rest of the Partitioner interface:

  • The NFS mounts were not displayed in the left tree (like it happens with all other devices).
  • It was impossible to use the menu to access to the NFS-related actions.
  • The NFS section of the Partitioner just behaved in a completely different way compared to all other tables (different buttons ordered and activated in a different way, forms in pop-ups instead of full-screen dialogs, validations at incorrect points of the workflow...)

Solution

This pull request goes together with yast/yast-nfs-client#105

With this pull request and the corresponding yast-nfs-client one mentioned above, the integration between both YaST modules is done in a more sensible way. All the functionality is still available, but in a way in which the integration is completely transparent offering a consistent user experience.

Testing

  • Tested manually
  • Adapted some new unit tests... BUT not as many as it would be desirable.
  • Rest of tests added at Tests for the recent code #1284 (we wanted to merge this PR because of a SLE-15-SP4 beta deadline, so we delayed the tests to that another PR).

@coveralls
Copy link

coveralls commented Feb 2, 2022

Coverage Status

Coverage decreased (-0.3%) to 97.423% when pulling 6467edb on ancorgs:nfs_form into 012b35c on yast:master.

@ancorgs ancorgs changed the title Use the Y2NfsClient::Widgets::NfsForm instead of NfsClient4Part Use Y2NfsClient::Widgets::NfsForm instead of NfsClient4Part Feb 2, 2022
ancorgs and others added 5 commits February 2, 2022 21:27
Co-authored-by: José Iván López González <jlopez@suse.com>
Co-authored-by: José Iván López González <jlopez@suse.com>
Co-authored-by: José Iván López González <jlopez@suse.com>
ancorgs and others added 2 commits February 2, 2022 21:52
Co-authored-by: José Iván López González <jlopez@suse.com>
Copy link
Contributor

@joseivanlopez joseivanlopez left a comment

Choose a reason for hiding this comment

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

LGTM

@ancorgs ancorgs merged commit 0786c0f into yast:master Feb 2, 2022
@yast-bot
Copy link

yast-bot commented Feb 2, 2022

✔️ Public Jenkins job #426 successfully finished
✔️ Created OBS submit request #950832

@yast-bot
Copy link

yast-bot commented Feb 2, 2022

✔️ Internal Jenkins job #219 successfully finished
✔️ Created IBS submit request #263911

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.

None yet

4 participants