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

ncm-sudo: Fix generation of parameter lists #465

Merged
merged 6 commits into from Apr 9, 2015

Conversation

jrha
Copy link
Member

@jrha jrha commented Apr 2, 2015

Specifying multiple options was possible, but untested by the unit tests, the delimiter was incorrectly specified as a tab character rather than a comma.

Fixes #460 as reported by @drossy.

jrha added 4 commits April 2, 2015 13:46
Rather than zero or more, as no whitespace would be invalid.
And get rid of all the damn tabs
As specified in the man page.
AFAIK tabs have never been an acceptable delimeter for parameter lists.

To test this, modify the all_options test to test parameter lists including an option of each type.

Fixes quattor#460.
@jrha jrha added this to the 15.4 milestone Apr 2, 2015
@hpcugentbot
Copy link

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/NCM_Components-pull_requests/608/
Test PASSed.

@hpcugentbot
Copy link

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/NCM_Components-pull_requests/609/
Test PASSed.

}
foreach my $o (INT_OPTS, STRING_OPTS) {
$ln .= "$o=" . $opts{$o}->getValue . ","
if defined $opts{$o};
Copy link
Member

Choose a reason for hiding this comment

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

indent

@stdweird
Copy link
Member

stdweird commented Apr 2, 2015

@jrha minor remarks, looks OK

This makes the generation of boolean and string options more similar.
Also add missing indent in logic of string options.
@jrha
Copy link
Member Author

jrha commented Apr 2, 2015

Thanks!

@hpcugentbot
Copy link

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/NCM_Components-pull_requests/610/
Test PASSed.

@stdweird
Copy link
Member

stdweird commented Apr 8, 2015

@jrha LGTM

ned21 added a commit that referenced this pull request Apr 9, 2015
ncm-sudo: Fix generation of parameter lists
@ned21 ned21 merged commit 7eb6c10 into quattor:master Apr 9, 2015
@jrha jrha deleted the sudo-fix-defaults branch April 15, 2015 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

ncm-sudo: generating incorrect Defaults lines with multiple options
4 participants