On Fri, Aug 19, 2016 at 12:54:07PM +0200, Petr Cech wrote:
> On 08/17/2016 02:37 PM, Petr Cech wrote:
> > On 08/12/2016 09:17 AM, Petr Cech wrote:
> > > On 08/11/2016 08:53 AM, Petr Cech wrote:
> > > > On 08/03/2016 12:34 PM, Michal Židek wrote:
> > > > > Two nitpicks, see inline.
> > > > >
> > > > > On 07/22/2016 02:34 PM, Petr Cech wrote:
> > > > > >
> > > > > > +static errno_t add_to_missing_attrs (TALLOC_CTX * mem_ctx,
> > > > > > + struct sysdb_attrs *attrs,
> > > > > > + const char *ext_key,
> > > > > > + char ***_missing)
> > > > > ^
> > > > > Coding style. Remove the space between function name and "(".
> > > > > Do not forget to align the parameters after that.
> > > >
> > > > Addressed.
> > > >
> > > > > > +{
> > > > > > + bool is_present = false;
> > > > > > + size_t size = 0;
> > > > > > + size_t ret;
> > > > > > +
> > > > > > + for (int i = 0; i < attrs->num; i++) {
> > > > > > + if (strcmp(ext_key, attrs->a[i].name) == 0) {
> > > > > > + is_present = true;
> > > > > > + }
> > > > > > + size++;
> > > > > > + }
> > > > > > +
> > > > > > + if (is_present == false) {
> > > > > > + ret = add_string_to_list(attrs, ext_key, _missing);
> > > > > > + if (ret != EOK) {
> > > > > > + goto fail;
> > > > > > + }
> > > > > > + }
> > > > > > +
> > > > > > + ret = EOK;
> > > > > > +
> > > > > > +fail:
> > > > >
> > > > > Please change the label name to "done". According to
> > > > > our new coding style, the code that follows label "fail"
> > > > > is only executed when failure occurs. I know we do not
> > > > > follow this everywhere, but I would like to be consistent
> > > > > in new code.
> > > >
> > > > Addressed.
> > > >
> > > > > > + return ret;
> > > > > > +}
> > > > > > +
> > > > >
> > > > > Other than that it looks good to me.
> > > > >
> > > > > Also it would be good to add a CI tests for this. I do
> > > > > not want to postpone this patch before release, so you can
> > > > > do it later as part of this ticket:
> > > > > https://fedorahosted.org/sssd/ticket/3119
> > > > >
> > > > > So either send a patch with CI test now or
> > > > > assign the above ticket to yourself and do it
> > > > > when there is more time.
> > > > >
> > > > > Michal
> > > >
> > > > Hello,
> > > >
> > > > there is fixed patch attached.
> > > >
> > > > I am working on CI test. I hope it will be ready soon.
> > > >
> > > > Regards.
> > >
> > > CI INTG tests will be solved in another mail thread. They are dependent
> > > on Lukas patches.
> >
> > Tests are solved in
> > [SSSD] [PATCH SET] WIP: INTG: Tests for ldap nested netgroups
> > mainling thread.
>
> Hello,
>
> whole patch set for
> LDAP: Fixing of removing netgroup from cache
> is in
> [SSSD] [PATCH SET] WIP: INTG: Tests for ldap nested netgroups
> mailing thread.
Can you change the thread subject then with the next response there? It's
a bit confusing that a fix is in WIP thread whose subject is tests..
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
