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

Finish simplifying NTP configuration using Settings #16393

Merged
merged 9 commits into from
Nov 27, 2017
Merged

Finish simplifying NTP configuration using Settings #16393

merged 9 commits into from
Nov 27, 2017

Conversation

bdunne
Copy link
Member

@bdunne bdunne commented Nov 3, 2017

Leverage Settings to simplify getting the NTP configuration from the Server / Zone / Region / Defaults

https://bugzilla.redhat.com/show_bug.cgi?id=1476365

@bdunne
No more need for server_ntp_settings
9dc07c5 
Everything is pulled from Settings now
https://bugzilla.redhat.com/show_bug.cgi?id=1476365
@bdunne
Use the default arg for ntp_reload
9bb69cc 
https://bugzilla.redhat.com/show_bug.cgi?id=1476365
@bdunne
NTP settings won't be blank and it doesn't matter where they're set
a49a4b8 
https://bugzilla.redhat.com/show_bug.cgi?id=1476365
@bdunne
These settings are no longer stored in the settings column
194f8a6 
https://bugzilla.redhat.com/show_bug.cgi?id=1476365
@bdunne
Move queue_ntp_reload_if_changed from a callback to a method
cb453e1 
We are no longer using the `settings` column on the record so the callback
is no longer valid.
https://bugzilla.redhat.com/show_bug.cgi?id=1476365
@bdunne
server_ntp_settings_blank? is no longer used, there's always a setting
3263499 
https://bugzilla.redhat.com/show_bug.cgi?id=1476365
@bdunne
Fetch the ntp_settings from within the method
e22c02d 
There is now only one caller, so no reason to be an independent method
https://bugzilla.redhat.com/show_bug.cgi?id=1476365
@bdunne
Update MiqServer NTP tests to not be concerned with testing Settings
b0e2a9f 
https://bugzilla.redhat.com/show_bug.cgi?id=1476365
@bdunne
Add a test to verify the file is only updated when settings have changed
bb78c4b 
https://bugzilla.redhat.com/show_bug.cgi?id=1476365
@bdunne bdunne requested a review from Fryguy November 3, 2017 21:06
@miq-bot
Copy link
Member

miq-bot commented Nov 3, 2017

Checked commits bdunne/manageiq@9dc07c5~...bb78c4b with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
5 files checked, 1 offense detected

spec/models/zone_spec.rb

@bdunne bdunne mentioned this pull request Nov 7, 2017
Merged
@Fryguy Fryguy merged commit b631e1a into ManageIQ:master Nov 27, 2017
@Fryguy Fryguy added this to the Sprint 74 Ending Nov 27, 2017 milestone Nov 27, 2017
@bdunne bdunne deleted the ntp_settings branch November 27, 2017 16:47
simaishi pushed a commit that referenced this pull request Nov 27, 2017
@Fryguy @simaishi
Merge pull request #16393 from bdunne/ntp_settings
19581f0 
Finish simplifying NTP configuration using Settings
(cherry picked from commit b631e1a)

https://bugzilla.redhat.com/show_bug.cgi?id=1517959
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 19581f06bed347d8e30b4408621eca0e435d176b
Author: Jason Frey <fryguy9@gmail.com>
Date:   Mon Nov 27 11:32:57 2017 -0500

    Merge pull request #16393 from bdunne/ntp_settings
    
    Finish simplifying NTP configuration using Settings
    (cherry picked from commit b631e1af2836401f2622a07cc0c07d7b33759c5b)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1517959

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carbonin carbonin carbonin approved these changes

@Fryguy Fryguy Awaiting requested review from Fryguy Fryguy is a code owner

Assignees

@gtanzillo gtanzillo

Labels
bug core gaprindashvili/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@bdunne @miq-bot @simaishi @carbonin @Fryguy @gtanzillo