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 Security Settings #150

Merged
merged 3 commits into from
Jun 21, 2021
Merged

Use Security Settings #150

merged 3 commits into from
Jun 21, 2021

Conversation

joseivanlopez
Copy link
Contributor

@joseivanlopez joseivanlopez commented Jun 17, 2021

Problem

AutoYaST client is still using Y2Firewall::ProposalSettings, but it should use Installation::SecuritySettings. Note that Installation::SecuritySettings was already adapted to use the new Y2Users API:

*yast/yast-installation#969

Solution

Adapt code to use Installation::SecuritySettings. Moreover, unused clients were removed: firewall_finish and firewall_proposal.

Test

  • Unit tests adapted
  • Manually tested

- firewall_finish and firewall_proposal
@coveralls
Copy link

coveralls commented Jun 21, 2021

Coverage Status

Coverage increased (+1.4%) to 94.955% when pulling ba6bce8 on joseivanlopez:y2users into b9e50c7 on yast:master.

Copy link
Member

@jreidinger jreidinger left a comment

Choose a reason for hiding this comment

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

+24 −1,325 is always good score for pr 👍

Copy link
Contributor

@teclator teclator 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 changed the title Adapt to Y2Users Use Security Settings Jun 21, 2021
@joseivanlopez joseivanlopez merged commit e19fff6 into yast:master Jun 21, 2021
@yast-bot
Copy link
Contributor

❌ Public Jenkins job #47 failed

@joseivanlopez joseivanlopez mentioned this pull request Jun 21, 2021
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

6 participants