On Thu, Nov 15, 2012 at 10:47:10AM -0800, Paul B. Henson wrote:
On 11/15/2012 5:45 AM, Stephen Gallagher wrote:
This is obviously already a significant enhancement, and of course the difference will be more pronounced for much larger environments. I'm prepared to give this an ack, with one comment to whoever pushes the patch upstream: please reflow the changes in nsssrv_cmd.c's fill_grent() that are now exceeding the 80-character line limit due to the new indentation.
Hmm, I see a few other lines in that file that exceed 80 characters; are there scenarios where there is an exception to that limit or were those just not caught ;)?
They slipped through the review, most probably. There's no hard rule, the 80-char limit just makes the code more readable for people like me who like to split their terminals vertically.
It looks like there were only two lines in my diff for that file that exceeded 80 characters, the attached updated patch should fix that.
Looks good to me, now. There's one more log line in groups_get_send but that one is a) hard to split, so it's a reasonable exception and b) that whole function doesn't respect the limit.
I tested the patch as well, works fine, the code looks great to me, to..
Thank you very much for the contribution!
Thanks for the ack. Will this just go into head or will it be pushed back to the 1.9 branch?
Currently just head, we might backport it later. I try to keep the 1.9 brach quite close to the RHEL6.4 codebase.
I've got an open RHEL support ticket requesting this be back ported to RHEL 6, given they're supposed to be shipping 1.9 in RHEL 6.4, it might be easier to get them to include it if it becomes part of the 1.9 branch :).
Yes, this is in fact the best way to go. Bugzillas coming from paying customers tend to get higher priority.
Or if whoever commits this "accidentally" slips it into the RHEL 6 repo while they're twiddling bits ;)...
Then my boss would "accidentally" fire me, I'm afraid :-)