From: Stephen Frost <sfrost@snowman.net>
To: KaiGai Kohei <kaigai@ak.jp.nec.com>
Cc: robertmhaas@gmail.com
Cc: pgsql-hackers@postgresql.org
Cc: kaigai@kaigai.gr.jp
Subject: Re: [PATCH] Reworks for Access Control facilities (r2350)
Date: Sun, 11 Oct 2009 22:46:04 -0400
Message-ID: <20091012024604.GW17756@tamriel.snowman.net> (raw)
In-Reply-To: <4AC5A14F.6050306@ak.jp.nec.com>
References: <4AC5A14F.6050306@ak.jp.nec.com>
KaiGai,
* KaiGai Kohei (kaigai@ak.jp.nec.com) wrote:
> Please review the new revision, Thanks,
In general, I'm pretty happy with this revision. You still have a
number of places where you have comments about code which does not exist
any more. For example, the comments about the check being removed from
LookupCreationNamespace. I would recommend pulling out those comments
and instead having a comment at the top of the function that says
"namespace creation permission checks are handled in the individual
object ac_*_create() routines".
I don't like having comments that are about code which was removed.
Some of these could be moved to the README if they aren't there already
and they really need to be kept.
There are some other grammatical and spelling issues in the comments,
but I don't believe any of this should hold this patch up from being
ready for committer. At a minimum, I think this really needs to have a
committer comment on it to ensure we're going in the right direction.
I'd be happy to continue working with KaiGai to review his changes going
forward, either with the next set of SE-PG patches or reworking this one
if necessary.
Thanks,
Stephen
Attachments:
signature.asc (application/pgp-signature, 196B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: sfrost@snowman.net, kaigai@ak.jp.nec.com, robertmhaas@gmail.com, kaigai@kaigai.gr.jp
Subject: Re: [PATCH] Reworks for Access Control facilities (r2350)
In-Reply-To: <20091012024604.GW17756@tamriel.snowman.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox