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

Fix iptables on CentOS6 + more tests for iptables (plus small code improvements) #442

Merged
merged 6 commits into from
Feb 10, 2016
Merged

Fix iptables on CentOS6 + more tests for iptables (plus small code improvements) #442

merged 6 commits into from
Feb 10, 2016

Conversation

srenatus
Copy link
Contributor

@srenatus srenatus commented Feb 9, 2016

  • unit tests: +1 test case including a comment
  • integration tests: haven't been there before

@srenatus srenatus changed the title More tests for iptables More tests for iptables (plus small code improvements) Feb 9, 2016
@srenatus srenatus mentioned this pull request Feb 9, 2016
Closed
@srenatus srenatus changed the title More tests for iptables (plus small code improvements) WIP More tests for iptables (plus small code improvements) Feb 9, 2016
@srenatus srenatus changed the title WIP More tests for iptables (plus small code improvements) More tests for iptables (plus small code improvements) Feb 9, 2016
@srenatus srenatus changed the title More tests for iptables (plus small code improvements) Fix iptables on CentOS6 + more tests for iptables (plus small code improvements) Feb 9, 2016
arlimus
arlimus reviewed Feb 10, 2016
View reviewed changes
lib/resources/iptables.rb
end

def retrieve_rules
return @iptables_cache if defined?(@iptables_cache)
return @iptables_cache if @iptables_cache
Copy link
Contributor

Choose a reason for hiding this comment

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

although ruby implicitly does the right thing, using defined? on a variable that has not been defined is better at expressing why this line is used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok reverted :)

@srenatus
iptables unit test: add comment examples
e184347 
this is not broken; but it should stay not broken ;)
@srenatus
iptables: add integration theses
81f149f
@srenatus
iptables: some simplifications
cdad6e6
@srenatus
iptables integration test: split according to platform
4ffc72b 
centos puts quotes where ubuntu drops them
@srenatus
iptables: strip lines if iptables -S output
ac2584f 
As it turns out, some of the lines on CentOS 6 had a trailing space in it.

Fixes #420.
@srenatus
iptables: adapt integration tests
7815cef 
_Only_ CentOS 6 does not strip the quotes from comments.
@arlimus
Copy link
Contributor

arlimus commented Feb 10, 2016

Awesome improvements, cheers @srenatus 👍 :) !

arlimus added a commit that referenced this pull request Feb 10, 2016
@arlimus
Merge pull request #442 from chef/sr/fix-420
575953b 
Fix iptables on CentOS6 + more tests for iptables (plus small code improvements)
@arlimus arlimus merged commit 575953b into master Feb 10, 2016
@arlimus arlimus deleted the sr/fix-420 branch February 10, 2016 09:02
@arlimus arlimus added the Type: Bug Feature not working as expected label Feb 15, 2016
@chris-rock chris-rock modified the milestone: 0.11.0 Feb 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Type: Bug Feature not working as expected
Projects
None yet
Milestone
0.11.0
Development

Successfully merging this pull request may close these issues.
4 participants
@srenatus @arlimus @chris-rock @chef-supermarket