On 29/10/14 17:23, David Kupka wrote:
On 10/29/2014 02:34 PM, David Kupka wrote:
On 10/24/2014 03:05 PM, David Kupka wrote:
On 10/24/2014 01:06 PM, David Kupka wrote:
On 10/24/2014 10:43 AM, Martin Basti wrote:
On 24/10/14 09:51, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/4585
NACK

1)
Why is there line with 'DS System User?' The comment should depend on
service.

+            args = [
+                paths.USERADD,
+                '-g', group,
+                '-c', 'DS System User',
+                '-d', homedir,
+                '-s', shell,
+                '-M', '-r', name,
+            ]

This was part of the original code and I didn't notice it. Nice catch,
thanks.


2)
code create_system_user is duplicated between base and redhat tasks
with
platform dependent changes.
IMO it would be better to have one method to create user, with keyword
arguments.  And then platform dependent method which will call
method to
create user with appropriate arguments (or with default arguments)


You're right it was ugly.



_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

I shouldn't break SOLID principles.



_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Using super is probably better that explicit naming of parent class.
Let user (developer) override UID/GID and hope that he knows why ...


_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel




In your former patch you had pki homedir path VAR_LIB_PKI_DIR :

+        if name == 'pkiuser':
+            uid = 17
+            gid = 17
+            homedir = paths.VAR_LIB_PKI_DIR
+            shell = paths.NOLOGIN
+            comment = 'CA System User'

in last patch you change it back to:

         homedir=paths.VAR_LIB,

so what is the correct path?

--
Martin Basti

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to