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

Extend Linux writer #323

Merged
merged 3 commits into from
Aug 12, 2021
Merged

Extend Linux writer #323

merged 3 commits into from
Aug 12, 2021

Conversation

joseivanlopez
Copy link
Contributor

@joseivanlopez joseivanlopez commented Aug 12, 2021

Problem

Right now, the Linux::Writer class only supports (auto)installation and firstboot use cases. During the installation, users can only be created. And for firstboot and auto-installation, users can be edited too. In firstboot, even the name of the user can be changed when you go back, but the Linux::Writer does not modify it in the system.

Moreover, the root password is not properly deleted if you go back to the wizard step for setting the root password and remove password (only an authorized key is selected). This affects to both: installation and firstboot.

Solution

Extend Linux::Writer in order to support the use cases mentioning above.

See also yast/yast-firstboot#127.

Testing

  • Added unit tests.
  • Manually tested.

@coveralls
Copy link

coveralls commented Aug 12, 2021

Pull Request Test Coverage Report for Build 1124306811

  • 14 of 16 (87.5%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.05%) to 53.623%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/lib/y2users/linux/users_writer.rb 11 13 84.62%
Files with Coverage Reduction New Missed Lines %
src/lib/users/dialogs/inst_user_first.rb 1 78.6%
Totals Coverage Status
Change from base Build 1056104890: 0.05%
Covered Lines: 2590
Relevant Lines: 4830

💛 - Coveralls

@joseivanlopez joseivanlopez changed the title Allow to edit user name Extend Linux writer Aug 12, 2021
Copy link
Member

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a lot.

Comment on lines +105 to +107
# FIXME: Validation methods should only check values. Note that with the current implementation,
# this method is taking care of setting/removing the user's password. This method should not
# modify the user.
Copy link
Member

Choose a reason for hiding this comment

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

👍 👍 👍

Copy link
Contributor

@shundhammer shundhammer left a comment

Choose a reason for hiding this comment

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

LGTM

@joseivanlopez joseivanlopez marked this pull request as ready for review August 12, 2021 16:04
@joseivanlopez joseivanlopez merged commit d4c2f99 into yast:master Aug 12, 2021
@yast-bot
Copy link
Contributor

✔️ Public Jenkins job #63 successfully finished
✔️ Created OBS submit request #911853

@yast-bot
Copy link
Contributor

✔️ Internal Jenkins job #25 successfully finished
✔️ Created IBS submit request #248294

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.

5 participants