Re: [5/5] usp10: remote todo_wine

[James Hawkins]

On 12/14/06, Clinton Stimpson <cjstimpson(a)utwire.net> wrote:
> Hi,
>
> Part 5 of 5.
> Remove many todo_wine's from the tests, now that the functions are
> implemented.

You have to remove the todo_wine's in the same patch that fixes the
tests, or the tests will fail for at least one commit.  Patches have
to be atomic and error free, and a failing test is an error.

-- 
James Hawkins

[5/5] usp10: remote todo_wine

[Clinton Stimpson]

Ok.  There are 4 functions that have to be implemented at the same time 
in order to not break any tests, because of how the tests were written.
A few days ago, I sent a single patch that implemented those 4 
functions, including an update of the tests.
It wasn't accepted, and it was suggested to break the patch up.  But, I 
can't break it up without breaking the tests.
I do have more patches coming after this batch, and those will be 
smaller and atomic.

So I guess I'm back to asking why my original patch wasn't accepted.  ??
Should I resend it?

Thanks,
Clinton

James Hawkins wrote:
> On 12/14/06, Clinton Stimpson <cjstimpson(a)utwire.net> wrote:
> > Hi,
> >
> > Part 5 of 5.
> > Remove many todo_wine's from the tests, now that the functions are
> > implemented.
>
> You have to remove the todo_wine's in the same patch that fixes the
> tests, or the tests will fail for at least one commit.  Patches have
> to be atomic and error free, and a failing test is an error.

[5/5] usp10: remote todo_wine

[Clinton Stimpson]

I can change the tests a bit, and change the currently empty functions 
to return E_NOTIMPL instead of S_OK.
Then I can do it piecemeal.
Is that how y'all want it?

Clint

Clinton Stimpson wrote:
> Ok.  There are 4 functions that have to be implemented at the same 
> time in order to not break any tests, because of how the tests were 
> written.
> A few days ago, I sent a single patch that implemented those 4 
> functions, including an update of the tests.
> It wasn't accepted, and it was suggested to break the patch up.  But, 
> I can't break it up without breaking the tests.
> I do have more patches coming after this batch, and those will be 
> smaller and atomic.
>
> So I guess I'm back to asking why my original patch wasn't accepted.  ??
> Should I resend it?
>
> Thanks,
> Clinton
>
> James Hawkins wrote:
> > On 12/14/06, Clinton Stimpson <cjstimpson(a)utwire.net> wrote:
> > > Hi,
> > >
> > > Part 5 of 5.
> > > Remove many todo_wine's from the tests, now that the functions are
> > > implemented.
> >
> > You have to remove the todo_wine's in the same patch that fixes the
> > tests, or the tests will fail for at least one commit.  Patches have
> > to be atomic and error free, and a failing test is an error.

[5/5] usp10: remote todo_wine

[Matt Finnicum]

I think that what James Hawkins had meant something like this:

Apply patch one, run the tests. If patch one fixed any of them, remove
the todo_wine's for those tests as a part of patch one. Otherwise,
leave them todo_wine.

Apply patch two, run the tests. If patch two fixed any of them, remove
the todo_wine's for those tests as a part of patch two. Etc.

This way, if someone builds wine at any step of applying the five
patches then the todo_wine's will reflect the current status of if the
functions work or not. I think this is important for regression
testing.

Of course, this will cause troubles if the patches are applied
out-of-order or the later patches are applied without the first ones -
but I think that's acceptable if you note that they should be
committed in order.

--Matt Finicum

On 12/15/06, Clinton Stimpson <cjstimpson(a)utwire.net> wrote:
> I can change the tests a bit, and change the currently empty functions
> to return E_NOTIMPL instead of S_OK.
> Then I can do it piecemeal.
> Is that how y'all want it?
>
> Clint
>
> Clinton Stimpson wrote:
> > Ok.  There are 4 functions that have to be implemented at the same
> > time in order to not break any tests, because of how the tests were
> > written.
> > A few days ago, I sent a single patch that implemented those 4
> > functions, including an update of the tests.
> > It wasn't accepted, and it was suggested to break the patch up.  But,
> > I can't break it up without breaking the tests.
> > I do have more patches coming after this batch, and those will be
> > smaller and atomic.
> >
> > So I guess I'm back to asking why my original patch wasn't accepted.  ??
> > Should I resend it?
> >
> > Thanks,
> > Clinton
> >
> > James Hawkins wrote:
> > > On 12/14/06, Clinton Stimpson <cjstimpson(a)utwire.net> wrote:
> > > > Hi,
> > > >
> > > > Part 5 of 5.
> > > > Remove many todo_wine's from the tests, now that the functions are
> > > > implemented.
> > >
> > > You have to remove the todo_wine's in the same patch that fixes the
> > > tests, or the tests will fail for at least one commit.  Patches have
> > > to be atomic and error free, and a failing test is an error.

[5/5] usp10: remote todo_wine

[James Hawkins]

On 12/15/06, Clinton Stimpson <cjstimpson(a)utwire.net> wrote:
> Ok.  There are 4 functions that have to be implemented at the same time
> in order to not break any tests, because of how the tests were written.
> A few days ago, I sent a single patch that implemented those 4
> functions, including an update of the tests.
> It wasn't accepted, and it was suggested to break the patch up.  But, I
> can't break it up without breaking the tests.
> I do have more patches coming after this batch, and those will be
> smaller and atomic.
>
> So I guess I'm back to asking why my original patch wasn't accepted.  ??
> Should I resend it?

I don't understand what the problem is.  If you implement a function
and a test now passes, remove the todo_wine from the test.  Similarly,
if the test now fails, add a todo_wine around the test (and explain
that a later patch in the series makes the test pass again).  There's
nothing wrong with sending an implementation of a function that
doesn't fix the tests, assuming later functions change that.

P.S. Please bottom-post on mailing lists, it makes the conversation
easier to read.

-- 
James Hawkins