[V2.1.1]: NNTP patch

["Noel J. Bergman" <no...@devtech.com>]

I have received a patch for NNTP with a number of corrections to fix
multiple issues:

  1) NEWNEWS command was wrong - didn't expect the distributions variable
  2) Log entry bug
  3) Added the missing article number in the XOVER
  4) Fixed article numbers to handle the case where an article is added to
an empty group.
     Note that this needs further work to handle articles being deleted from
groups
  5) Aggressively nulled out entries in the spooler loop
  6) Added finally clauses to ensure streams are closed
  7) Added a close to deal with a race condition for spool file deletion

My plan is to apply these after reviewing and testing, which should be next
week.  If anyone plans to work on NNTP, please try using these patches
first.

	--- Noel

Re: [V2.1.1]: NNTP patch

[Aaron Knauf <ak...@xtra.co.nz>]

Noel J. Bergman wrote:
>>There are in my mind a number issues with the current code.
>>There are parts of refactoring that make things difficult
>>as we go forward. I will be happy elaborate post tests.
> 
> 
> Why don't you elaborate now?  What specific technical issues do you have
> with the current code?
> 

FWIW, I agree that the current bug reports are too vague.

I know absolutely nothing about NNTP, but from reading the posts 
pertaining to this issue, I gather that the MODE and XOVER commands are 
broken.  How they are broken, and the effect that this has is not made 
clear.

Including the following information in the bug report would (IMHO) make 
the discussions on this subject more productive:

1)	A brief description of the error, including the specific wording of 
any error messages displayed.
2)	Instructions on how to reproduce the error.
3)	A description of the effect that this has on the application's 
operation. (To be used in determining the priority given to fixing the 
problem.)


Of particular concern to me is the fact that there is only one bug 
description (vague as it is) supplied, while there is consistent 
allusion to multiple bugs.  The above information should be supplied 
*for each* bug.

> As for the patches I have, I have submitted them for review, I do plan to
> test them, and if they work, I see no reason why we should not use them.
> But I am also willing to see what you have in mind.

As far as I can see, only one set of fixes has been proposed.  There are 
three options from here:

1)  If the fixes work +1 them and get on with it.
2)  If the fixes don't work, or break something else, -1 them.
3)  If the fixes do work, but you don't like the implementation, suggest 
a better alternative.  If you don't have a better alternative, (this 
need not be fully working code, just suggestions) then let the fix go in 
and refactor later.

As for a test suite, this is very desirable.  I could tell many success 
stories about the usefullness of JUnit test suites during code refactoring.

However, in the absence of a test suite, I don't think anyone really 
wants the NNTP code to stay broken until someone gets around to writing 
one.  Especially with a fix already submitted.

JUnit tests are simply a way of automating the regression testing 
process.  It ought to be relatively simple to verify this fix through 
manual testing.

That was my 2 shekels worth, anyway.

Cheers

ADK



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

Re: [V2.1.1]: NNTP patch

[Harmeet Bedi <ha...@kodemuse.com>]

----- Original Message -----
From: "Noel J. Bergman" <no...@devtech.com>
> > The NNTP patch is valuable and good, but preempting an already taken up
> job
> > is not the right thing to do.
>
> Are you planning to patch the existing code or replace with a new version
of
> your old code?

I don't intend a rollback. I had suggested it earlier because it was the
safest thing to days before the release but given that the changes are going
in a point release, I have no such intent/desire.

I do think there are problems with both the refactoring that went in and the
process(no consensus, refactoring with misleading comments, no proposal, no
branch) but I don't want to veto the changes and rollback. Causes too much
ache to be worth it. I wish and hope a better process is followed for
refactoring in future. I will enumerate the problems that I think are there,
but may not have to stomach to do more.

Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

RE: [V2.1.1]: NNTP patch

["Noel J. Bergman" <no...@devtech.com>]

> The NNTP patch is valuable and good, but preempting an already taken up
job
> is not the right thing to do.

Are you planning to patch the existing code or replace with a new version of
your old code?

	--- Noel


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

Re: [V2.1.1]: NNTP patch

[Harmeet Bedi <ha...@kodemuse.com>]

> I see no reason not to fix what we have if it passes the tests.

Agreed,

The NNTP patch is valuable and good, but preempting an already taken up job
is not the right thing to do.

Please hold off on the checkins, I have it integrated. Please continue to
test and so will I. I want to reach a comfort level and do it next week or
by weekend. This patch will be there so no need to preempt or push a point
esp as we both are thinking the same thing(high quality and fixed code) and
time frame(next week).

Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

RE: [V2.1.1]: NNTP patch

["Noel J. Bergman" <no...@devtech.com>]

Harmeet,

> The raft has mostly been due to refactoring. As a comparison the first
> checkin of NNTP was working better than current code.

The raft of errors I refered to was in the original code that needed to be
fixed.  The current version of the code appears to have relatively trivial
errors that may indeed have broken the protocol, but are nonetheless minor
in terms of the code change.

I see no reason not to fix what we have if it passes the tests.

	--- Noel


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

Re: [V2.1.1]: NNTP patch

[Harmeet Bedi <ha...@kodemuse.com>]

----- Original Message -----
From: "Noel J. Bergman" <no...@devtech.com>
> > There are in my mind a number issues with the current code.
> > There are parts of refactoring that make things difficult
> > as we go forward. I will be happy elaborate post tests.
>
> Why don't you elaborate now?  What specific technical issues do you have
> with the current code?

It is a distraction. I expect to be done with tests and all fixes in 1 week
or at most 10 days. Shorter say 3 days if there an aggressive need to hurry
up and get 2.1.1 out.

I could say a few things and start a email chain and what I say may not be
complete as I haven't tested sufficiently.

>
> > This is exactly what happenned in the past when I had patches but
> > before I could patch, there were untested pre-emptive checkins.
>
> Again?  We keep going over this.  There were no check-ins to NNTP for many
> months despite a raft of defects in the code.
>
> As for the patches I have, I have submitted them for review, I do plan to
> test them, and if they work, I see no reason why we should not use them.
> But I am also willing to see what you have in mind.

The raft has mostly been due to refactoring. As a comparison the first
checkin of NNTP was working better than current code.
All I have in mind is that regressions should be fixed and there should
verifiable tests. That would make everybody on this list more sane and help
avoid regresssions in future. I have spent the new year holiday thinking
about a good testing strategy and researching a few libraries. I would not
have done that if I felt we should just do the fixes. It was working
earlier, got broken, caused a lot of ache, don't want it repeated. Let us
take this as a learning experience and not rush to fix things.

If the time frame offered is not good enough or you think I am being
unreasonable please go ahead, otherwise I'll be happy to get NNTP working
with tests that I belive that would please everybody here and James
community.

Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

RE: [V2.1.1]: NNTP patch

["Noel J. Bergman" <no...@devtech.com>]

> There are in my mind a number issues with the current code.
> There are parts of refactoring that make things difficult
> as we go forward. I will be happy elaborate post tests.

Why don't you elaborate now?  What specific technical issues do you have
with the current code?

> This is exactly what happenned in the past when I had patches but
> before I could patch, there were untested pre-emptive checkins.

Again?  We keep going over this.  There were no check-ins to NNTP for many
months despite a raft of defects in the code.

As for the patches I have, I have submitted them for review, I do plan to
test them, and if they work, I see no reason why we should not use them.
But I am also willing to see what you have in mind.

	--- Noel


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

Re: [V2.1.1]: NNTP patch

[Harmeet Bedi <ha...@kodemuse.com>]

----- Original Message -----
From: "Noel J. Bergman" <no...@devtech.com>
> My plan is to apply these after reviewing and testing, which should be
next
> week.

I plan to work on this. There are in my mind a number issues with the
current code. There are parts of refactoring that make things difficult as
we go forward. I will be happy elaborate post tests.

Give me some time. Let us first have working verifiable tests and then have
fixes. Making patches/fixes is very easy, but seems more short term than
doing sufficient testing. I have a set of patches too but don't want to even
want to think about it before having tests in place. This is exactly what
happenned in the past when I had patches but before I could patch, there
were untested pre-emptive checkins. Let us not repeat this please.
Tests before checkin, esp. given that the we have had to disable a working
(for end users. There were bugs to be sure but also successful users)
protocol in 2.1.

Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>