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

Add SLES10.x support #96

Merged
merged 1 commit into from
Feb 2, 2015
Merged

Conversation

propyless
Copy link

SLES10.x does not support limits fragments as
it does not traverse the limits.d catalog.


- *Default*: undef

config_file_source
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think we need to support source, since we support contents, unless you really want this. Removing will make testing easier.

@propyless propyless force-pushed the sles10_support branch 2 times, most recently from 4001a92 to a0f2ec2 Compare February 2, 2015 07:00
@propyless
Copy link
Author

Hello! Phil has written the spec tests and checks source and contents etc. Since the support for source is already in the code it feels a bit unnecessary to remove it for the sake of simplicity. I've updated the README as it the description was a bit thin as pointed out by you :).

Hopefully this travis build will pass =)

@@ -355,6 +355,19 @@ Mode for config_file.

- *Default*: '0640'

config_file_contents
Copy link
Owner

Choose a reason for hiding this comment

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

The word contents suggests the contents of the file, such as providing all of the contents, when this is actually an array of lines. For clarity, could this be changed to config_file_lines ?

@propyless
Copy link
Author

Yes I can agree with you on that as well. Fixed. =)

@ghoneycutt
Copy link
Owner

@Phil-Friderici This look good to you?

SLES10.x does not support limits fragments as
it does not traverse the limits.d catalog.
@Phil-Friderici
Copy link
Contributor

@ghoneycutt: yes functionality looks good to me. Maybe the helper variable $content could be renamed to $config_file_content, but that's not a variable that users come in touch with.

@Phil-Friderici
Copy link
Contributor

@propyless: you are fixing fast ;)

@propyless
Copy link
Author

Whats a guy gotta do to get a merge around here :P

@ghoneycutt
Copy link
Owner

Lulz. Looks good, though I'm not merging anything so late at night. Will check it out again tomorrow.

@Phil-Friderici
Copy link
Contributor

@ghoneycutt Thanks & have a good night !

@propyless
Copy link
Author

Sleep tight

ghoneycutt added a commit that referenced this pull request Feb 2, 2015
@ghoneycutt ghoneycutt merged commit 0798d05 into ghoneycutt:master Feb 2, 2015
@ghoneycutt
Copy link
Owner

Released in v2.14.0

@ghoneycutt
Copy link
Owner

Thanks gents

@Phil-Friderici
Copy link
Contributor

@ghoneycutt @propyless was a pleasure :)

@propyless
Copy link
Author

Thanks all =).

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.

3 participants