[PATCH 1 of 1] [ACL] Added support for OS-level groups (using '@group' syntax)

[Elifarley Callado Coelho Cruz]

Hi Nicolas, thanks for your tips :)

I really thought I had inlined my patch, so I'll check to see what went
wrong about that.

Regarding the coding style, I will rewrite my patch so that it more closely
follows it.


> On the patch itself:

* _I_ would rather have you filter once the list, separate groups /

users in two clear lists and use them later, instead of using two

comprehensions.


Hmm.. Nice, I'll do that.



> * introducing a grp import means that the extension is now Unix only

(was it ever multiplatform? I do not know, but we might care)


Should I put that unix-specific method in util.py ?
Anyways, the extension will be unix-only just if you use groups.
And even then, you'll be able to make it OS-agnostic if you specify your
groups in a special section of the .hgrc file (I'll provide a patch for that
later)




> * What happens if grp.getgrname() raises a KeyError?


The function is interrupted. Suggestions?



On Mon, Apr 26, 2010 at 09:32, Nicolas Dumazet <nicdumz at gmail.com> wrote:

> Hello!
>
> 2010/4/26 elifarley <elifarley at gmail.com>:
> >
> > Oh, the changeset can also be found at:
> > http://bitbucket.org/elifarley/mercurial-crew/changeset/67f28e83f6dd
>
> Thanks for your patch!
>
>
> I'll comment mostly on the form, because I'm not qualified to review
> the idea / am not familiar with this extension in particular
>
>
> The patch is likely to receive complaints, because it does not comply
> to http://mercurial.selenic.com/wiki/BasicCodingStyle We dislike, in
> particular, underscores.
>
> On the patch itself:
> * _I_ would rather have you filter once the list, separate groups /
> users in two clear lists and use them later, instead of using two
> comprehensions.
> * introducing a grp import means that the extension is now Unix only
> (was it ever multiplatform? I do not know, but we might care)
> * What happens if grp.getgrname() raises a KeyError?
>
> For your next patch, please send it to us using the patchbomb
> extension, _inlining_ your patch in the mail body:
> http://mercurial.selenic.com/wiki/ContributingChanges
> It's not (only) about following annoying arbitrary constraints, it's
> because it allows reviewers to quote directly the patch content,
> speeding up reviews and discussions.
>
> Cheers,
>
> --
> Nicolas Dumazet — NicDumZ
>



-- 
Elifarley Cruz

Profile: http://bit.ly/9hrz0P
Professional info: http://br.linkedin.com/in/elifarley
Google Reader: http://bit.ly/2W7JK2
Bookmarks: http://delicious.com/elifarley
http://twitter.com/elifarley
http://elifarley.amplify.com/
-

" Do not believe anything because it is said by an authority, or if it  is
said to come from angels, or from Gods, or from an inspired source.
Believe it only if you have explored it in your own heart and mind and body
and found it to be true.  Work out your own path, through diligence."
- Gautama Buddha
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20100426/f5ec3c6c/attachment.htm>