Trevor Watson wrote: > Bart Smaalders wrote: >> http://cr.opensolaris.org/~barts/user+group_actions/ >> >> >> Add user and group actions.... >> >> more will be added later (user_attr attributes in particular), >> but this makes upgrades much nicer for now. > > Only some minor stuff as far as I can see Bart... > > group.py: > Lines 65,77,88: > To be consistent with user.py, should you import * from pkg.cfgfiles > and use the unqualified name for GroupFile on these lines?
yes.... done > > user.py: > Lines 126-129: > Are these print statements just for debugging? If so, should they be > removed before checkin? > whoops.... the test harness hides output on success. > Lines 137,141: > Would it be possible to wrap these in a test of the value of the > 'ftpuser' attribute or do you envisage a situation where some > earlier call could add a user to ftpusers, and a later call would > say to not have this user in ftpusers? This works now w/ adding and removing ftpuser capability. What were you trying to achieve? > > cfgfiles.py: > Line 190: > I'm not sure I can think of a good reason why anyone would set ACLs > on these files, but if they did, the ACL's would be not be > preserved. Interesting... we have the same problem elsewhere. I'll think about this. > > Line 211: > There appears to be an extra indentation on this line. I wonder how that happened... thanks! > > Do you think there is any need to keep track of when the file was read > just in case something else writes the file after we have read it, but > before we write it back? > Another something to mull over.... I need file compare and swap :-). > Lines 440-: > I guess this is debugging/test. Do you need to keep it? > We're keeping these embedded tests for now... > Trev Thanks! - Bart -- Bart Smaalders Solaris Kernel Performance [EMAIL PROTECTED] http://blogs.sun.com/barts "You will contribute more with mercurial than with thunderbird." _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
