Re: [SSSD] [PATCHES] Ding libs patches for merge related functionality

[Dmitri Pal]

On 12/05/2012 08:28 AM, Ondrej Kos wrote:
>  On 05/12/12 02:17, Dmitri Pal wrote:
> > Hello,
> >
> > Please find attached the list of the 10 patches for ding-libs.
> > These patches implement merge capabilities of the ding-libs and refine
> > parser to consistently deal with different parse modes.
> >
> >
> > Patches 1-5 are self contained patches that can be applied independently
> > from each other but in the given order.
> > After applying each of those the tree should build-able.
> >
> > Patches 6-10 are related to each other and need to be viewed and applied
> > in relation. They are a logically split for an easier review one piece
> > of functionality .
> >
> > Patch names and commit comments give an idea of what happens in each
> > patch.
> > Most complicated patches are 6, 7 and 10.
> >
> > Let me know if I missed something.
> >
> >
> >
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel(a)lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >
>
>  [PATCH 1/10]
>
>  *ACK*
>
>
>  [PATCH 2/10]
>
>  just a nitpick, I wouldn't delete the empty line after introductory
>  comment (maybe leave it for the future code re-styling)
>
>  @@ -18,7 +18,6 @@
>       You should have received a copy of the GNU Lesser General Public
>  License
>       along with Collection Library.  If not, see
>  <http://www.gnu.org/licenses/>;.
>   */
>  -
>   #define _GNU_SOURCE
>   #include <string.h>
>   #include <stdlib.h>
>
>  otherwise *ACK* 
A new patch is ready.

>  [PATCH 3/10]
>
>  git am doesn't like the two empty lines added to the end of file 
I can't find place where there are two lines at the end of the file.
>  $ gam 0003-INI-Merge-and-serialize-comments.patch
>  Applying: Merge and serialize comments
>  /home/okos/devel/ding-libs/.git/rebase-apply/patch:89: trailing
>  whitespace.
>                             struct simplebuffer *sbobj)
>  warning: 1 line adds whitespace errors. 
That I found and fixed.
Will be in the next set of patches.

>  and *NACK*:
>
>  ini_comment_serialize doesn't work as expected
>  it is appending multiple CRs, not sure when exactly, but it probably
>  has something to do with multi-line comments. and those empty lines
>  result into
>  = =
>  thus breaking config file
>  example:
>
>  #Hoho section
>  [hoho]
>  #Hoho value
>  val = hoho
>  #End of hoho
>  #Start of section
>  [foo]
>  #Second value
>  bar = second value
>  #End of section
>  = =
>  #Hoho section
>  [hoho]
>  #Hoho value
>  val = hoho
>  #Hoho section
>  [hoho]
>  #Hoho value
>  val = hoho
>  #End of hoho
>
>  
Are you looking at the ini_parse_ut?
Hm let me try applying these patches one by one.
May be I need to restructure unit tests

>  [PATCH 4/10]
>
>  make-check fails on:
>
>  Files ./merge.conf.out and ./expect.conf.out differ
>  Failed to run copy command 256 1.
>  FAIL: ini_parse_ut
>
>  caused by PATCH 3 
Same thing

>  [PATCH 5/10]
>
>  *Ack* (i noticed the warning with the first patch)
>
>
>
>  review for patches 6-10 will follow.
>
>  Ondra
>  
I will send the refined patch set shortly

-- 
Thank you,
Dmitri Pal

Sr. Engineering Manager for IdM portfolio
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/