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

Write restrict records first (bsc#983486) #86

Merged
merged 3 commits into from Jul 3, 2017
Merged

Write restrict records first (bsc#983486) #86

merged 3 commits into from Jul 3, 2017

Conversation

teclator
Copy link
Contributor

@teclator teclator commented Jun 30, 2017 •
edited

Trello card: https://trello.com/c/N9tffDE4/1062-sles12-sp2-p3-983486-yast2-ntp-client-mangles-etc-ntpconf

Default template is something like:

# Comment
# Comment
# Comment
restrict -4 default notrap...
restrict -6 default notrap...

pool.ntp.example.org

And after read and write it was written as:

pool.ntp.example.org
# Comment
# Comment
# Comment
restrict -4 default notrap...
restrict -6 default notrap...

This PR just writes the restrict entries before the rest.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 65.474% when pulling 2f97065 on teclator:write_restrict_first into f627002 on yast:SLE-12-SP2.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 65.474% when pulling b47c5af on teclator:write_restrict_first into f627002 on yast:SLE-12-SP2.

teclator
teclator commented Jul 3, 2017
View reviewed changes
src/modules/NtpClient.rb
@ntp_records += restrict_map_records
# Restrict map records are written first to not mangle the config file
# (bsc#983486)
@ntp_records = restrict_map_records + @ntp_records
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't like that it adds restrict maps to ntp_records each time Write is called, we probably could remove this line and just concatenate both arrays in records_for_write, but is preferable to do it in a separate PR.

joseivanlopez
joseivanlopez approved these changes Jul 3, 2017
View reviewed changes
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

@teclator teclator merged commit b9e05ac into yast:SLE-12-SP2 Jul 3, 2017
@teclator teclator deleted the write_restrict_first branch July 3, 2017 07:59
This was referenced Jul 3, 2017
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joseivanlopez joseivanlopez joseivanlopez approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@teclator @coveralls @joseivanlopez