[PATCH] Merging loadparm further
Garming Sam
garming at catalyst.net.nz
Wed Jul 30 20:48:20 MDT 2014
Thanks a lot Michael.
I've looked over the changes you've made and they all seemed pretty
reasonable.
Cheers,
Garming
On 29/07/14 22:58, Michael Adam wrote:
> Hi Garming,
>
> I am now done reviewing your patches.
> Attached find my reviewed version. I:
> - adapted a few commit messages
> - fixed one formatting
> - split one commit in 2
> - replaced your comment fix by my comment removal
> - added one loadparm patch on top
>
> I indicated in the commit message by "(OBNOX:...)" where I changed
> something.
>
> I'm happy with thos changes pushed to master.
> My two (trivial) patches would need a team-review.
>
> The patches can also be found in my master-loadparm branch:
>
> git://git.samba.org/obnox/samba/samba-obnox.git
>
> https://git.samba.org/?p=obnox/samba/samba-obnox.git;a=shortlog;h=refs/heads/master-loadparm
>
> Cheers - Michael
>
>
> On 2014-07-26 at 02:20 +0200, Michael Adam wrote:
>> Hi Garming,
>>
>> nice progress!
>>
>> On 2014-07-25 at 14:33 +1200, Garming Sam wrote:
>>> Hi there,
>>>
>>> I've run these patches through the usual tests and made sure each
>>> patch passes the main tests we identified. It also passes the full
>>> autobuild with the winbindd patches in my other mail.
>>>
>>> This set of patches actually makes some good progress in merging the
>>> two codebases. I managed to remove a reasonable chunk of code.
>>>
>>> lib/param/loadparm.c | 239 ++++++++++++++++++++++++++----------
>>> lib/param/loadparm.h | 1 -
>>> lib/param/s3_param.h | 5 -
>>> source3/param/loadparm.c | 414
>>> +++++++++-----------------------------------------------------
>>> source3/param/loadparm_ctx.c | 5 -
>>> 5 files changed, 235 insertions(+), 429 deletions(-)
>>>
>>>
>>> Changes from now on should hopefully be quite a bit easier than it
>>> was back when I started.
>>>
>>> In particular, performing a 'do_parameter' is now equivalent and
>>> setting defaults could be done from the same code.
>>>
>>> A review would be appreciated.
>> I started to look at the patches, and while
>> the general train of thought is obvious and
>> the patches are clear, and most of them look
>> good. But I would like to verify
>> in a few places that the replaced calls are
>> really equivalend.
>>
>>
>> One trivial comment right away:
>>
>>> >From 8d0a409e41728dd26b580d4bc722e4ebbe02efc4 Mon Sep 17 00:00:00 2001
>>> From: Garming Sam <garming at catalyst.net.nz>
>>> Date: Fri, 21 Mar 2014 09:36:04 +1300
>>> Subject: [PATCH 09/28] lib/param: fix comment in lpcfg_service_ok
>>>
>>> Change-Id: I111836be6d556e0e63c1e4129216f01c74e5e0d5
>>> Signed-off-by: Garming Sam <garming at catalyst.net.nz>
>>> Reviewed-by: Andrew Bartlett <abartlet at samba.org>
>>> ---
>>> lib/param/loadparm.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
>>> index 5aaef13..71ee4ad 100644
>>> --- a/lib/param/loadparm.c
>>> +++ b/lib/param/loadparm.c
>>> @@ -964,7 +964,7 @@ bool lpcfg_service_ok(struct loadparm_service *service)
>>> service->bAvailable = false;
>>> }
>>>
>>> - /* If a service is flagged unavailable, log the fact at level 0. */
>>> + /* If a service is flagged unavailable, log the fact at level 1. */
>>> if (!service->bAvailable)
>>> DEBUG(1, ("NOTE: Service %s is flagged unavailable.\n",
>>> service->szService));
>>> --
>>> 1.8.3.2
>> I'd rather remove that comment altogehter: It is redundant.
>> Alternative patch attached. ;-)
>>
>> Cheers - Michael
>>
>>
>> From 039412b071ec12492ec1163b88ae45c6e8220335 Mon Sep 17 00:00:00 2001
>> From: Michael Adam <obnox at samba.org>
>> Date: Sat, 26 Jul 2014 02:17:07 +0200
>> Subject: [PATCH] lib/param: remove a redundant (and wrong) comment from
>> lpcfg_service_ok()
>>
>> Signed-off-by: Michael Adam <obnox at samba.org>
>> ---
>> lib/param/loadparm.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
>> index b58a058..cfee591 100644
>> --- a/lib/param/loadparm.c
>> +++ b/lib/param/loadparm.c
>> @@ -964,7 +964,6 @@ static bool lpcfg_service_ok(struct loadparm_service *service)
>> service->browseable = false;
>> }
>>
>> - /* If a service is flagged unavailable, log the fact at level 0. */
>> if (!service->bAvailable)
>> DEBUG(1, ("NOTE: Service %s is flagged unavailable.\n",
>> service->szService));
>> --
>> 1.9.1
>>
>
>
More information about the samba-technical
mailing list