Please Review: Managed Entries Plug-in

[Nathan Kinder]

Please hold off on the review.  I noticed one other thing that I need to 
address.  A new patch will be sent out shortly.

-NGK

On 03/30/2010 01:18 PM, Nathan Kinder wrote:
> --
> 389-devel mailing list
> [e-mail]
> https://admin.fedoraproject.org/mailman/listinfo/389-devel

[Nathan Kinder]

New patch attached.

-NGK

On 03/30/2010 01:24 PM, Nathan Kinder wrote:
> Please hold off on the review.  I noticed one other thing that I need 
> to address.  A new patch will be sent out shortly.
>
> -NGK
>
> On 03/30/2010 01:18 PM, Nathan Kinder wrote:
> > --
> > 389-devel mailing list
> > [e-mail]
> > https://admin.fedoraproject.org/mailman/listinfo/389-devel
>
> --
> 389-devel mailing list
> [e-mail]
> https://admin.fedoraproject.org/mailman/listinfo/389-devel

[Rich Megginson]

Nathan Kinder wrote:
> New patch attached.

Nice!

mep.c:491 - value is not freed if it is not a valid filter - also would 
be nice to have the error message print value to show what the bogus 
filter is

Do we plan on supporting binary attrs?  e.g.
mepstaticattr: blobattr:: base64value
not sure if we care about this for the first iteration
> -NGK
>
> On 03/30/2010 01:24 PM, Nathan Kinder wrote:
> > Please hold off on the review.  I noticed one other thing that I need 
> > to address.  A new patch will be sent out shortly.
> >
> > -NGK
> >
> > On 03/30/2010 01:18 PM, Nathan Kinder wrote:
> > > --
> > > 389-devel mailing list
> > > [e-mail]
> > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> >
> > --
> > 389-devel mailing list
> > [e-mail]
> > https://admin.fedoraproject.org/mailman/listinfo/389-devel
>
> --
> 389-devel mailing list
> [e-mail]
> https://admin.fedoraproject.org/mailman/listinfo/389-devel

[Nathan Kinder]

On 03/30/2010 03:44 PM, Rich Megginson wrote:
> Nathan Kinder wrote:
> > New patch attached.
>
> Nice!

Thanks for the review!
> mep.c:491 - value is not freed if it is not a valid filter - also would
> be nice to have the error message print value to show what the bogus
> filter is

I'll address this.
> Do we plan on supporting binary attrs?  e.g.
> mepstaticattr: blobattr:: base64value
> not sure if we care about this for the first iteration

This could be nice for static attributes.  I'm not really sure if it 
makes sense for mapped attributes or not.  I'll look into how easily 
this could be done, but it is something we could add later as well.

-NGK
> > -NGK
> >
> > On 03/30/2010 01:24 PM, Nathan Kinder wrote:
> > > Please hold off on the review.  I noticed one other thing that I need
> > > to address.  A new patch will be sent out shortly.
> > >
> > > -NGK
> > >
> > > On 03/30/2010 01:18 PM, Nathan Kinder wrote:
> > > > --
> > > > 389-devel mailing list
> > > > [e-mail]
> > > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> > >
> > > --
> > > 389-devel mailing list
> > > [e-mail]
> > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> >
> > --
> > 389-devel mailing list
> > [e-mail]
> > https://admin.fedoraproject.org/mailman/listinfo/389-devel
>
> --
> 389-devel mailing list
> [e-mail]
> https://admin.fedoraproject.org/mailman/listinfo/389-devel

[Rich Megginson]

Nathan Kinder wrote:
> On 03/30/2010 03:44 PM, Rich Megginson wrote:
> > Nathan Kinder wrote:
> > > New patch attached.
> >
> > Nice!
>
> Thanks for the review!
> > mep.c:491 - value is not freed if it is not a valid filter - also would
> > be nice to have the error message print value to show what the bogus
> > filter is
>
> I'll address this.
> > Do we plan on supporting binary attrs?  e.g.
> > mepstaticattr: blobattr:: base64value
> > not sure if we care about this for the first iteration
>
> This could be nice for static attributes.  I'm not really sure if it 
> makes sense for mapped attributes or not.  I'll look into how easily 
> this could be done, but it is something we could add later as well.

Ok.  If we don't need it now, we can defer this until later.
> -NGK
> > > -NGK
> > >
> > > On 03/30/2010 01:24 PM, Nathan Kinder wrote:
> > > > Please hold off on the review.  I noticed one other thing that I need
> > > > to address.  A new patch will be sent out shortly.
> > > >
> > > > -NGK
> > > >
> > > > On 03/30/2010 01:18 PM, Nathan Kinder wrote:
> > > > > --
> > > > > 389-devel mailing list
> > > > > [e-mail]
> > > > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> > > >
> > > > --
> > > > 389-devel mailing list
> > > > [e-mail]
> > > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> > >
> > > --
> > > 389-devel mailing list
> > > [e-mail]
> > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> >
> > --
> > 389-devel mailing list
> > [e-mail]
> > https://admin.fedoraproject.org/mailman/listinfo/389-devel
>
> --
> 389-devel mailing list
> [e-mail]
> https://admin.fedoraproject.org/mailman/listinfo/389-devel

[Nathan Kinder]

On 03/31/2010 09:02 AM, Rich Megginson wrote:
> Nathan Kinder wrote:
> > On 03/30/2010 03:44 PM, Rich Megginson wrote:
> > > Nathan Kinder wrote:
> > > > New patch attached.
> > >
> > > Nice!
> >
> > Thanks for the review!
> > > mep.c:491 - value is not freed if it is not a valid filter - also would
> > > be nice to have the error message print value to show what the bogus
> > > filter is
> >
> > I'll address this.
> > > Do we plan on supporting binary attrs?  e.g.
> > > mepstaticattr: blobattr:: base64value
> > > not sure if we care about this for the first iteration
> >
> > This could be nice for static attributes.  I'm not really sure if it
> > makes sense for mapped attributes or not.  I'll look into how easily
> > this could be done, but it is something we could add later as well.
>
> Ok.  If we don't need it now, we can defer this until later.

I've attached a new patch that addresses the leak of the filter string, 
prints out the bad filter in the error message, and handles binary 
static values in the managed entry templates.

-NGK
> > -NGK
> > > > -NGK
> > > >
> > > > On 03/30/2010 01:24 PM, Nathan Kinder wrote:
> > > > > Please hold off on the review.  I noticed one other thing that I need
> > > > > to address.  A new patch will be sent out shortly.
> > > > >
> > > > > -NGK
> > > > >
> > > > > On 03/30/2010 01:18 PM, Nathan Kinder wrote:
> > > > > > --
> > > > > > 389-devel mailing list
> > > > > > [e-mail]
> > > > > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> > > > >
> > > > > --
> > > > > 389-devel mailing list
> > > > > [e-mail]
> > > > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> > > >
> > > > --
> > > > 389-devel mailing list
> > > > [e-mail]
> > > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> > >
> > > --
> > > 389-devel mailing list
> > > [e-mail]
> > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> >
> > --
> > 389-devel mailing list
> > [e-mail]
> > https://admin.fedoraproject.org/mailman/listinfo/389-devel
>
> --
> 389-devel mailing list
> [e-mail]
> https://admin.fedoraproject.org/mailman/listinfo/389-devel

[Rich Megginson]

Nathan Kinder wrote:
> On 03/31/2010 09:02 AM, Rich Megginson wrote:
> > Nathan Kinder wrote:
> > > On 03/30/2010 03:44 PM, Rich Megginson wrote:
> > > > Nathan Kinder wrote:
> > > > > New patch attached.
> > > >
> > > > Nice!
> > >
> > > Thanks for the review!
> > > > mep.c:491 - value is not freed if it is not a valid filter - also 
> > > > would
> > > > be nice to have the error message print value to show what the bogus
> > > > filter is
> > >
> > > I'll address this.
> > > > Do we plan on supporting binary attrs?  e.g.
> > > > mepstaticattr: blobattr:: base64value
> > > > not sure if we care about this for the first iteration
> > >
> > > This could be nice for static attributes.  I'm not really sure if it
> > > makes sense for mapped attributes or not.  I'll look into how easily
> > > this could be done, but it is something we could add later as well.
> >
> > Ok.  If we don't need it now, we can defer this until later.
>
> I've attached a new patch that addresses the leak of the filter 
> string, prints out the bad filter in the error message, and handles 
> binary static values in the managed entry templates.

looks good - ack
> -NGK
> > > -NGK
> > > > > -NGK
> > > > >
> > > > > On 03/30/2010 01:24 PM, Nathan Kinder wrote:
> > > > > > Please hold off on the review.  I noticed one other thing that I 
> > > > > > need
> > > > > > to address.  A new patch will be sent out shortly.
> > > > > >
> > > > > > -NGK
> > > > > >
> > > > > > On 03/30/2010 01:18 PM, Nathan Kinder wrote:
> > > > > >
> > > > > > > -- 
> > > > > > > 389-devel mailing list
> > > > > > > [e-mail]
> > > > > > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > >
> > > > > > 389-devel mailing list
> > > > > > [e-mail]
> > > > > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> > > > >
> > > > > -- 
> > > > > 389-devel mailing list
> > > > > [e-mail]
> > > > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> > > >
> > > > -- 
> > > > 389-devel mailing list
> > > > [e-mail]
> > > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> > >
> > > -- 
> > > 389-devel mailing list
> > > [e-mail]
> > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> >
> > -- 
> > 389-devel mailing list
> > [e-mail]
> > https://admin.fedoraproject.org/mailman/listinfo/389-devel
>
> --
> 389-devel mailing list
> [e-mail]
> https://admin.fedoraproject.org/mailman/listinfo/389-devel

[Nathan Kinder]

On 04/01/2010 06:04 PM, Rich Megginson wrote:
> Nathan Kinder wrote:
> > On 03/31/2010 09:02 AM, Rich Megginson wrote:
> > > Nathan Kinder wrote:
> > > > On 03/30/2010 03:44 PM, Rich Megginson wrote:
> > > > > Nathan Kinder wrote:
> > > > > > New patch attached.
> > > > >
> > > > > Nice!
> > > >
> > > > Thanks for the review!
> > > > > mep.c:491 - value is not freed if it is not a valid filter - also
> > > > > would
> > > > > be nice to have the error message print value to show what the bogus
> > > > > filter is
> > > >
> > > > I'll address this.
> > > > > Do we plan on supporting binary attrs?  e.g.
> > > > > mepstaticattr: blobattr:: base64value
> > > > > not sure if we care about this for the first iteration
> > > >
> > > > This could be nice for static attributes.  I'm not really sure if it
> > > > makes sense for mapped attributes or not.  I'll look into how easily
> > > > this could be done, but it is something we could add later as well.
> > >
> > > Ok.  If we don't need it now, we can defer this until later.
> >
> > I've attached a new patch that addresses the leak of the filter
> > string, prints out the bad filter in the error message, and handles
> > binary static values in the managed entry templates.
>
> looks good - ack

Thanks.  Pushed to master.

Counting objects: 27, done.
Delta compression using 2 threads.
Compressing objects: 100% (16/16), done.
Writing objects: 100% (16/16), 18.56 KiB, done.
Total 16 (delta 10), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
    4b520bf..1f56658  master -> master
> > -NGK
> > > > -NGK
> > > > > > -NGK
> > > > > >
> > > > > > On 03/30/2010 01:24 PM, Nathan Kinder wrote:
> > > > > >
> > > > > > > Please hold off on the review.  I noticed one other thing that I
> > > > > > > need
> > > > > > > to address.  A new patch will be sent out shortly.
> > > > > > >
> > > > > > > -NGK
> > > > > > >
> > > > > > > On 03/30/2010 01:18 PM, Nathan Kinder wrote:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >                
> > > > > > >> -- 
> > > > > > >> 389-devel mailing list
> > > > > > >> [e-mail]
> > > > > > >> https://admin.fedoraproject.org/mailman/listinfo/389-devel
> > > > > > >>
> > > > > > >>
> > > > > > >>
> > > > > > >>                  
> > > > > > > -- 
> > > > > > > 389-devel mailing list
> > > > > > > [e-mail]
> > > > > > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > >
> > > > > > -- 
> > > > > > 389-devel mailing list
> > > > > > [e-mail]
> > > > > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> > > > >
> > > > > -- 
> > > > > 389-devel mailing list
> > > > > [e-mail]
> > > > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> > > >
> > > > -- 
> > > > 389-devel mailing list
> > > > [e-mail]
> > > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> > >
> > > -- 
> > > 389-devel mailing list
> > > [e-mail]
> > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> >
> > --
> > 389-devel mailing list
> > [e-mail]
> > https://admin.fedoraproject.org/mailman/listinfo/389-devel
>
> --
> 389-devel mailing list
> [e-mail]
> https://admin.fedoraproject.org/mailman/listinfo/389-devel

[Noriko Hosoi]

(2010?03?30? 13:56), Nathan Kinder wrote:
> New patch attached.

Ack.
--noriko
> -NGK
>
> On 03/30/2010 01:24 PM, Nathan Kinder wrote:
> > Please hold off on the review.  I noticed one other thing that I need 
> > to address.  A new patch will be sent out shortly.
> >
> > -NGK
> >
> > On 03/30/2010 01:18 PM, Nathan Kinder wrote:
> > > --
> > > 389-devel mailing list
> > > [e-mail]
> > > https://admin.fedoraproject.org/mailman/listinfo/389-devel
> >
> > --
> > 389-devel mailing list
> > [e-mail]
> > https://admin.fedoraproject.org/mailman/listinfo/389-devel
>
> --
> 389-devel mailing list
> [e-mail]
> https://admin.fedoraproject.org/mailman/listinfo/389-devel