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

Strip whitespaces in netgroup triple. #568

Closed

Conversation

malyzelenyhnus
Copy link
Contributor

Strip leading and trailing whitespaces from netgroup three-tuple
strings to be compatible with nss_ldap.

@centos-ci
Copy link

Can one of the admins verify this patch?

1 similar comment
@centos-ci
Copy link

Can one of the admins verify this patch?

@fidencio
Copy link
Contributor

fidencio commented May 7, 2018

okay to test

@jhrozek
Copy link
Contributor

jhrozek commented May 8, 2018

ok to test

@jhrozek
Copy link
Contributor

jhrozek commented May 28, 2018

Just a quick note because the PR did not get any review for three weeks now -- I checked the code when it was submitted and it looks OK after a visual inspection. I was going to write a test to make sure splitting the triples works, but I (and others on our team) were swamped with unexpected urgent work, so I did not have the time to finish the test.

So, I'm sorry this PR got stalled, it's on my list to get back to it. Thank you very much for the contribution.

@jhrozek
Copy link
Contributor

jhrozek commented Jul 19, 2018

I'm sorry it took me two months to get to the review. I found one minor issue where you left some unused variables after the refactoring and wrote a unit test. Both commits are in my netgrtest branch. If you agree, could you cherry-pick and squash the patch that removes the unused variables? Then I'd push your patch and submit the test separately.

@jhrozek
Copy link
Contributor

jhrozek commented Jul 19, 2018

removing variables: jhrozek@a6fd80a
unit test:
jhrozek@ca91f4f

@jhrozek
Copy link
Contributor

jhrozek commented Jul 20, 2018

btw I'm setting changes requested to this PR because the unused variables should be squashed. I can do that myself as well if you don't have the time (I hope you didn't get too irritated about the long response time in the meantime..)

Strip leading and trailing whitespaces from netgroup three-tuple
strings to be compatible with nss_ldap.
@jhrozek
Copy link
Contributor

jhrozek commented Jul 30, 2018

Thank you, ack

@jhrozek jhrozek self-assigned this Jul 30, 2018
@jhrozek
Copy link
Contributor

jhrozek commented Jul 30, 2018

@jhrozek jhrozek closed this Jul 30, 2018
@jhrozek jhrozek added the Pushed label Jul 30, 2018
@jhrozek
Copy link
Contributor

jhrozek commented Jul 30, 2018

Thank you very much for the contribution and sorry for the huge delay in review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants