-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Make pam.d directory configurable #2552
Conversation
bb38927
to
0a11593
Compare
Hi @yifanjiang I'm out of time this week to have a good look at this, but after a quick glance this indeed looks like I was thinking of. This will definitely be useful to SuSE and any other distros that don't use A quick question- is there a good reason why you didn't add Speak soon. |
f502974
to
5b43f6a
Compare
Hi @matt335672 , thanks for looking into this. The reason was to make it a little more flexible for downstream packaging. While I took a second look, keeping a consistent lexical style with socketdir and sesmanruntimedir might be a better choice. So I update through the changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with this now. Waiting for @matt335672 's approval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was all ready to approve this, then I read this useful comment from @thkukuk
I suggest the following amendment.
@yifanjiang - I apologise for messing you about like this. However, what we've now got something that should be able to support the use-cases mentioned by @thkuk
8ef999e
to
9faea3c
Compare
Thanks for the quick edits @yifanjiang Again, I apologise for the confusion. |
@metalefty - I think this should be backported to v0.9. Do you agree? |
@matt335672 Yes, it is definitely useful. |
Change according to the discussion in #2547 .