Added Dns class for managing DNS search and nameservers #127
Conversation
lib/linux_admin/dns.rb
Outdated
There was a problem hiding this comment.
The split feels like it belongs in #parse_file
There was a problem hiding this comment.
I would expect #reload to clear the attr_accessors
There was a problem hiding this comment.
parse_file clears them to start. Would it look better to clear them first here?
carbonin
force-pushed
the
add_dns_class
branch
from
8f2b251 to
57703c2
Compare
spec/dns_spec.rb
Outdated
There was a problem hiding this comment.
We should try to avoid writing files in a spec, especially files that hold test data. In the spec, you should be able to:
allow(File).to receive(:read).and_return(original_content, new_content)
carbonin
force-pushed
the
add_dns_class
branch
from
57703c2 to
0136080
Compare
spec/dns_spec.rb
Outdated
There was a problem hiding this comment.
You can expect(contents).to include(new_search_order)
|
@carbonin Sorry for all the comments, but I really like where this is going. 👍 |
carbonin
force-pushed
the
add_dns_class
branch
from
0136080 to
e7c84f6
Compare
carbonin
force-pushed
the
add_dns_class
branch
from
e7c84f6 to
7ff854d
Compare
|
Checked commit carbonin@7ff854d with ruby 1.9.3, rubocop 0.34.2, and haml-lint 0.13.0 |
There was a problem hiding this comment.
These two tests could also be merged in a followup PR
|
👍 LGTM, Thanks! |
Added Dns class for managing DNS search and nameservers
3 participants
@bdunne @Fryguy Please review
https://trello.com/c/rWKh4KQs