Re: platform with minzip, commit or not commit.

[jan i <ja...@apache.org>]

On 31 December 2014 at 20:41, Dennis E. Hamilton <de...@acm.org>
wrote:

>  -- replying below to --
> From: jan i [mailto:jani@apache.org]
> Sent: Wednesday, December 31, 2014 09:55
> To: dev@corinthia.incubator.apache.org; Dennis Hamilton
> Subject: Re: platform with minzip, commit or not commit.
>
> On 31 December 2014 at 18:46, Dennis E. Hamilton <de...@acm.org>
> wrote:
>
> >  -- replying below to --
> > From: jan i [mailto:jani@apache.org]
> > Sent: Wednesday, December 31, 2014 08:28
> > To: dev@corinthia.incubator.apache.org
> > Subject: platform with minzip, commit or not commit.
> >
> > Hi All.
> >
> > I am in serious doubt. I have completed the isolation of minzip with a
> > platform api in wrapper.c
> >
> > BUT we have no test cases for DFZipFile.c, so I cannot really be sure I
> > have done it all correct.
> >
> > <orcmid>
> >    I assume that you haven't broken the build.
> >    So the question is, what is the policy on making untested changes?
> >
> >    ESPECIALLY when a new API is being introduced.
> >
> +1, this is really something we need to form a policy about.....I am scared
> about what a stray commit can do.
>
>
> >
> >    I would do this in two stages.
> >
> >     1. Do not depend on the wrapper yet, and do not build with it. (If
> >    this creates CMake problems, keep the wrapper on a branch.  This
> >    might also help stay out of the way of changes that Kelly is making.)
> >
> >    Commit the wrapper so you are not stuck with uncommitted changes when
> >    you want to sync with the repository.
> >
> >    Allow the wrapper to be reviewed and any tests for it to be devised
> >    before modifying other code to use the wrapper.
> >
> I cannot follow you here. I cannot commit the changes without the wrapper,
> then it will not build, secondly we have no place where I can make
> wrapper.c available apart from the development branch.
>
> <orcmid>
>    I don't understand why you have to commit this on the development
> branch.
>    I would have thought that you'd create a new repository branch
>    specifically for introducing this and having it reviewed and tested.
>    Any changes to CMakeFile.txt would be on the branch too.
>
Sometimes I simply think to complicated....I was somehow fixed on that we
only have 2 remote branches, I have created a temporary branch for the
review.


>
>    (I have the sneaking suspicion that I will make myself crazy dealing
>    with CMake.)
>
Welcome in the club, it was one of the first discussions I had with peter,
but he convinced me of the benefits.

I still believe we should have visual studio solution checked in, so that
windows developers are up an running in no time.



>
> >
> >     2. When cutover to using the wrapper API happens, do that without
> other
> >    changes.  Since the direct use of minizip worked, any failures can
> >    be isolated to introduction of the Wrapper API.  This will be a lot
> >    easier to identify and handle.
> >
> That is exactly what I have prepared now.
>
> Wrapper.c + DFZipFile.c (the only consumer) and the needed CMakeFile.txt
> changes.
>
> Errors can be in either DFZipFile.c or wrapper.c. Since I moved code from
> one into the other I need to commit both at the same time.
>
> But I listen carefully to the arguments, because this is a really important
> matter for the future. Of course I believe it works, but so will any
> developer in the future, and we are all just humans.
>
> thanks for commenting.
>
> <orcmid>
>    And have you come to a conclusion?
> </orcmid>
>
yes you will see a request for review in a moment.

rgds
jan I.


> rgds
> jan i.
>
>
> > </orcmid>
> >
> > I am on my way writing test cases for platform in general, but that will
> > take another week, and I would like to commit the minizip changes, since
> it
> > involved a number of CMakeFiles.txt.  Be aware the platform test cases
> will
> > not ensure that DFZipFile.c works.
> >
> > What is the general opinion, should I run the risk and push to master
> > (hoping you all will help catch the problems, and not put me in the
> > dungeon) or should I wait until someone provides DFZipFile.c test cases
> > (which should normally be the case).
> >
> > Once I have completed test cases for platform, it is my intention to
> > continue with core, but start with a long discussion.
> >
> > rgds
> > jan i
> >
> >
>
>