Re: [PATCH] note acl

On 17/05/2013 4:41 a.m., Tsantilas Christos wrote:
> Hi all,

Sorry for the slow reply..

> I am attaching two patches here:
>
> 1) Merge AccessLogEntry 'helperNotes' and 'configNotes' members to
> 'notes' member (trunk-merge-cfg-helper-notes-v2.patch)
>
> There is not any need to store notes added using Note cfg option and
> notes added from helper to separated member. This patch merge them to
> the same AccessLogEntry::note member.

+1. Looks fine as-is.

PS. When you apply this please prefix the commit subject with "Cleanup: "

> 2) note acl ( trunk-note-ACL-v5.patch )
> Syntax:
> acl aclname note name [value ...]
>
> Without values, matches any annotation with a given name. With value(s),
> matches any annotation with a given name that also has one of the given
> values. Annotation sources include note and adaptation_meta directives
> as well as helper and eCAP responses.

audit results for the second patch:

* please do not cut-n-paste the (partially wrong) copyright from other
files into new ones.
  NOTE: a default Squid copyright blurb is planned to be automatically
pre-pended somehow. So for now only add one if you want one different
from the main Squid COPYRIGHT file. To mark the copyright as yours for
this file under the default, add your name in an AUTHOR: line instead
for now.

* please also add a "note_regex" ACL which takes regex patterns as the
value (and/or key) details.
  NP: that will allow removing "Names and values are compared using a
string equality test." statement in cf.data.pre

in: src/acl/NoteData.h:
* please be consistent about doxygen comments:
   + "///" for one-liners before the member definition
   + "///<" for one-liners after the member definition.
   + "/**" for multi-liners before the member definition
   + _no_ multi-liners after the member definition.

* you don't need to document each #include.
   We are only using that to document why the files like
globals.h/enums.h/defines.h/structs.h which are scheduled for removal
are still being included.

Amos
Received on Tue May 21 2013 - 06:14:23 MDT