fidian
commented
7 years ago It looks like your last commit only covered a small fraction of the open concerns. I'm listing them here in one big comment so you can meticulously address them individually.
Missed, wick-make-group, line 7:
... I think this whole sentence should be stricken and instead wickMakeUser should automatically create groups when the group doesn't exist.
And later I wrote this:
Not sure if you got the point I was trying to make with the second sentence, so I shall reword. I don't think that you should say the user should use this in conjunction with wickMakeUser as that should be automatic. Maybe you could change the phrasing to say that wickMakeUser uses this function to manage groups.
Missed, wick-make-group, line 12:
You amended this to say "a unix group" but Unix is a proper noun and deserves a capital U. Or you can omit the descriptor entirely and say "Create a group".
New, wick-make-user, line 24:
My grammar OCD is kicking in and you should add a comma after "used", then change "is added" into "are added".
Missed, wick-make-user, line 26:
Both useradd and usermod should have backticks. They aren't English words and are instead code.
Missed, wick-make-user, line 48:
Please alphabetize the variables.
And later:
Missed this one.
Missed, wick-make-user, line 79:
Please add a newline above.
A lot of this is covered in the style guide, cited in the CONTRIBUTING.md file.
New, wick-make-user, lines 107 and 142:
Indentation is off again. This is the third indentation problem noticed. You're not using the EditorConfig plugin with your favorite text editor, which makes this whitespace stuff trivial and fixes things for you.
Edit Removed the last one. It should not have been included.
withnale
A couple of my review comments are in the "outdated" code, but still apply. Make sure to check them all out.