You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@corinthia.apache.org by jan i <ja...@apache.org> on 2014/12/31 17:27:53 UTC
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.
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
Re: platform with minzip, commit or not commit.
Posted by 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
> >
> >
>
>
RE: platform with minzip, commit or not commit.
Posted by "Dennis E. Hamilton" <de...@acm.org>.
-- 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.
(I have the sneaking suspicion that I will make myself crazy dealing
with CMake.)
>
> 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>
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
>
>
Re: platform with minzip, commit or not commit.
Posted by jan i <ja...@apache.org>.
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.
>
> 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.
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
>
>
RE: platform with minzip, commit or not commit.
Posted by "Dennis E. Hamilton" <de...@acm.org>.
-- 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.
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.
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.
</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