VOTE: Release commons-fileupload 1.2

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

VOTE: Release commons-fileupload 1.2

jochen-2
Hi,

after the release of commons-parent, the hurdles for publishing the
next version of commons-fileupload have now gone. So I'd like to
request a release of

    Commons Fileupload 1.2

The proposed distributables can be found at

    http://people.apache.org/~jochen/commons-fileupload/dist

The proposed web site is at

    http://people.apache.org/~jochen/commons-fileupload/site

Notable changes in 1.2 are:

- Made Streams.asString static. Thanks to Aaron Freeman.
- Eliminated duplicate code. (FILEUPLOAD-109)
- Added a streaming API. (FILEUPLOAD-112)
- Eliminated the necessity of a content-length header. (FILEUPLOAD-93)
- Eliminated the limitation of a maximum size for a single header
line. (FILEUPLOAD-108)
  Thanks to Amichai Rothman.
- Added the ProgressListener, which allows to implement a progress bar.
  (FILEUPLOAD-87)
- Added support for header continuation lines. (FILEUPLOAD-111)
  Thanks to Amichai Rothman.
- It is now possible to limit the actual file size and not the request size.
  (FILEUPLOAD-88) Thanks to Andrey Aristarkhov.


Please cast your vote:

[] +1
[] =0
[] -1


Jochen

--
My wife Mary and I have been married for forty-seven years and not
once have we had an argument serious enough to consider divorce;
murder, yes, but divorce, never.
(Jack Benny)

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: VOTE: Release commons-fileupload 1.2

Martin Cooper-3
On 11/15/06, Jochen Wiedmann <[hidden email]> wrote:
>
> Hi,
>
> after the release of commons-parent, the hurdles for publishing the
> next version of commons-fileupload have now gone. So I'd like to
> request a release of
>
>     Commons Fileupload 1.2


I'm not going to have time to look at this until the weekend, but I will try
to spend some time on it then. In the meantime, a couple of requests:

1) Given the significant API changes since 1.1.1, I would like to see clirr
and jdiff reports, so that we can verify backwards compatibility more
reliably.

2) I would like to see the output of Robert's RAT tool, run over the
proposed distro, to make sure were not missing anything.

--
Martin Cooper


The proposed distributables can be found at

>
>     http://people.apache.org/~jochen/commons-fileupload/dist
>
> The proposed web site is at
>
>     http://people.apache.org/~jochen/commons-fileupload/site
>
> Notable changes in 1.2 are:
>
> - Made Streams.asString static. Thanks to Aaron Freeman.
> - Eliminated duplicate code. (FILEUPLOAD-109)
> - Added a streaming API. (FILEUPLOAD-112)
> - Eliminated the necessity of a content-length header. (FILEUPLOAD-93)
> - Eliminated the limitation of a maximum size for a single header
> line. (FILEUPLOAD-108)
>   Thanks to Amichai Rothman.
> - Added the ProgressListener, which allows to implement a progress bar.
>   (FILEUPLOAD-87)
> - Added support for header continuation lines. (FILEUPLOAD-111)
>   Thanks to Amichai Rothman.
> - It is now possible to limit the actual file size and not the request
> size.
>   (FILEUPLOAD-88) Thanks to Andrey Aristarkhov.
>
>
> Please cast your vote:
>
> [] +1
> [] =0
> [] -1
>
>
> Jochen
>
> --
> My wife Mary and I have been married for forty-seven years and not
> once have we had an argument serious enough to consider divorce;
> murder, yes, but divorce, never.
> (Jack Benny)
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: VOTE: Release commons-fileupload 1.2

jochen-2
On 11/16/06, Martin Cooper <[hidden email]> wrote:

> 1) Given the significant API changes since 1.1.1, I would like to see clirr
> and jdiff reports, so that we can verify backwards compatibility more
> reliably.

I have created a clirr report on

    http://people.apache.org/~jochen/commons-fileupload/clirr.txt

Haven't got jdiff to work.


> 2) I would like to see the output of Robert's RAT tool, run over the
> proposed distro, to make sure were not missing anything.

    http://people.apache.org/~jochen/commons-fileupload/bin-rat.txt
    http://people.apache.org/~jochen/commons-fileupload/src-rat.txt

--
My wife Mary and I have been married for forty-seven years and not
once have we had an argument serious enough to consider divorce;
murder, yes, but divorce, never.
(Jack Benny)

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: VOTE: Release commons-fileupload 1.2

Niall Pemberton
In reply to this post by jochen-2
On 11/15/06, Jochen Wiedmann <[hidden email]> wrote:

> Hi,
>
> after the release of commons-parent, the hurdles for publishing the
> next version of commons-fileupload have now gone. So I'd like to
> request a release of
>
>     Commons Fileupload 1.2
>
> The proposed distributables can be found at
>
>     http://people.apache.org/~jochen/commons-fileupload/dist
>
> The proposed web site is at
>
>     http://people.apache.org/~jochen/commons-fileupload/site
>
> Notable changes in 1.2 are:
>
> - Made Streams.asString static. Thanks to Aaron Freeman.
> - Eliminated duplicate code. (FILEUPLOAD-109)
> - Added a streaming API. (FILEUPLOAD-112)
> - Eliminated the necessity of a content-length header. (FILEUPLOAD-93)
> - Eliminated the limitation of a maximum size for a single header
> line. (FILEUPLOAD-108)
>   Thanks to Amichai Rothman.
> - Added the ProgressListener, which allows to implement a progress bar.
>   (FILEUPLOAD-87)
> - Added support for header continuation lines. (FILEUPLOAD-111)
>   Thanks to Amichai Rothman.
> - It is now possible to limit the actual file size and not the request size.
>   (FILEUPLOAD-88) Thanks to Andrey Aristarkhov.
>
>
> Please cast your vote:
>
> [] +1
> [] =0
> [] -1

I would have preferred the release be cut using maven1 rather than m2.
The maven1 build is tried and tested and the gripes of those of us
that checked out previous releases have been fixed in the maven1
build. I guess that doesn't matter if the release is up to scratch but
would be interested to know if others think we're ready for releases
using m2 yet?

The first issue I have with checking out this RC is that you've only
posted the "tar.gz" source and binary distros. I would have liked to
see the full set - zip versions and md5 checksums (the maven1 build
for fileupload creates the md5 checksums for you).

I think there are three serious issues with this RC:
1) It doesn't comply with the new "ASF Source Header and Copyright
Notice Policy":
    http://www.apache.org/legal/src-headers.html

2) According to the jar's manifest file its been built using JDK
1.6.0-rc. Even if JDK 1.6 had been (just) released using it for a
release would make me nervous - but using a RC version of Java to cut
the fileupload release is bad news IMO.

3) The clirr report you produced: which shows the following
incompatibilities with the previous fileupload version:

1) FileUploadBase - public constant MAX_HEADER_SIZE removed.
2) FileUploadBase - protected method createItem removed
3) FileUploadBase - two public constructors for
SizeLimitExceededException removed
4) FileUploadBase - public static class UnknownSizeException removed
5) MulitpartStream - gone from public to package visibility

For these reasons my vote is -1.

One thing thats disappointing is that the last fileupload release had
only 5 checkstyle issues. This one has 376 of which 200 are for tab
characters which I personally dislike in source.

Looking at the source distro "rat" report for the nightly build - its
showing 7 missing license headers - the 2 that stand out are for the
test cases: FileUploadTestCase and ProgressListenerTest:

  http://vmbuild.apache.org/~commons/nightly/nightly_reports/commons-fileupload/src-rat.txt

Niall

> Jochen

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: VOTE: Release commons-fileupload 1.2

Niall Pemberton
I've updated the source file headers to comply with the new policy:

  http://svn.apache.org/viewvc?view=rev&revision=479262

Also could you update your subversion client to automatically add
subversion properties when you add new artifacts - the main one being
svn:eol-style. Hopefully I've added svn properties for all the new
files you've added:

   http://svn.apache.org/viewvc?view=rev&revision=479261

More info on svn properties and configuring your client here:

   http://wiki.apache.org/struts/StrutsMaintenanceSvn

Niall

On 11/26/06, Niall Pemberton <[hidden email]> wrote:

> On 11/15/06, Jochen Wiedmann <[hidden email]> wrote:
> > Hi,
> >
> > after the release of commons-parent, the hurdles for publishing the
> > next version of commons-fileupload have now gone. So I'd like to
> > request a release of
> >
> >     Commons Fileupload 1.2
> >
> > The proposed distributables can be found at
> >
> >     http://people.apache.org/~jochen/commons-fileupload/dist
> >
> > The proposed web site is at
> >
> >     http://people.apache.org/~jochen/commons-fileupload/site
> >
> > Notable changes in 1.2 are:
> >
> > - Made Streams.asString static. Thanks to Aaron Freeman.
> > - Eliminated duplicate code. (FILEUPLOAD-109)
> > - Added a streaming API. (FILEUPLOAD-112)
> > - Eliminated the necessity of a content-length header. (FILEUPLOAD-93)
> > - Eliminated the limitation of a maximum size for a single header
> > line. (FILEUPLOAD-108)
> >   Thanks to Amichai Rothman.
> > - Added the ProgressListener, which allows to implement a progress bar.
> >   (FILEUPLOAD-87)
> > - Added support for header continuation lines. (FILEUPLOAD-111)
> >   Thanks to Amichai Rothman.
> > - It is now possible to limit the actual file size and not the request size.
> >   (FILEUPLOAD-88) Thanks to Andrey Aristarkhov.
> >
> >
> > Please cast your vote:
> >
> > [] +1
> > [] =0
> > [] -1
>
> I would have preferred the release be cut using maven1 rather than m2.
> The maven1 build is tried and tested and the gripes of those of us
> that checked out previous releases have been fixed in the maven1
> build. I guess that doesn't matter if the release is up to scratch but
> would be interested to know if others think we're ready for releases
> using m2 yet?
>
> The first issue I have with checking out this RC is that you've only
> posted the "tar.gz" source and binary distros. I would have liked to
> see the full set - zip versions and md5 checksums (the maven1 build
> for fileupload creates the md5 checksums for you).
>
> I think there are three serious issues with this RC:
> 1) It doesn't comply with the new "ASF Source Header and Copyright
> Notice Policy":
>     http://www.apache.org/legal/src-headers.html
>
> 2) According to the jar's manifest file its been built using JDK
> 1.6.0-rc. Even if JDK 1.6 had been (just) released using it for a
> release would make me nervous - but using a RC version of Java to cut
> the fileupload release is bad news IMO.
>
> 3) The clirr report you produced: which shows the following
> incompatibilities with the previous fileupload version:
>
> 1) FileUploadBase - public constant MAX_HEADER_SIZE removed.
> 2) FileUploadBase - protected method createItem removed
> 3) FileUploadBase - two public constructors for
> SizeLimitExceededException removed
> 4) FileUploadBase - public static class UnknownSizeException removed
> 5) MulitpartStream - gone from public to package visibility
>
> For these reasons my vote is -1.
>
> One thing thats disappointing is that the last fileupload release had
> only 5 checkstyle issues. This one has 376 of which 200 are for tab
> characters which I personally dislike in source.
>
> Looking at the source distro "rat" report for the nightly build - its
> showing 7 missing license headers - the 2 that stand out are for the
> test cases: FileUploadTestCase and ProgressListenerTest:
>
>   http://vmbuild.apache.org/~commons/nightly/nightly_reports/commons-fileupload/src-rat.txt
>
> Niall
>
> > Jochen
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: VOTE: Release commons-fileupload 1.2

jochen-2
In reply to this post by Niall Pemberton
Hi, Niall,

On 11/26/06, Niall Pemberton <[hidden email]> wrote:

> I would have preferred the release be cut using maven1 rather than m2.
> The maven1 build is tried and tested and the gripes of those of us
> that checked out previous releases have been fixed in the maven1
> build. I guess that doesn't matter if the release is up to scratch but
> would be interested to know if others think we're ready for releases
> using m2 yet?

See for example

    http://marc.theaimsgroup.com/?l=jakarta-commons-dev&m=115714503628112&w=2
    http://marc.theaimsgroup.com/?t=115719054100004



> The first issue I have with checking out this RC is that you've only
> posted the "tar.gz" source and binary distros. I would have liked to
> see the full set - zip versions and md5 checksums (the maven1 build
> for fileupload creates the md5 checksums for you).

Are you actually going to tell me, that this is an issue? (The archive
type, not the checksum, which must of course, be added to
distributables. As are .sha1 and .asc files, btw.) If so, changing it
is as simple as adding one line to the assembly descriptor. But who do
you believe is unable to use tar.gz in 2006?


> I think there are three serious issues with this RC:
> 1) It doesn't comply with the new "ASF Source Header and Copyright
> Notice Policy":
>     http://www.apache.org/legal/src-headers.html

I wasn't aware of a change in policy. You are right, that must be
honoured. However, you already did it, so it cannot be counted as a
reason for a -1, right?


> 2) According to the jar's manifest file its been built using JDK
> 1.6.0-rc. Even if JDK 1.6 had been (just) released using it for a
> release would make me nervous - but using a RC version of Java to cut
> the fileupload release is bad news IMO.

Ok, I'll keep that in mind for the next approach.



> 3) The clirr report you produced: which shows the following
> incompatibilities with the previous fileupload version:
>
> 1) FileUploadBase - public constant MAX_HEADER_SIZE removed.
> 2) FileUploadBase - protected method createItem removed
> 3) FileUploadBase - two public constructors for
> SizeLimitExceededException removed
> 4) FileUploadBase - public static class UnknownSizeException removed
> 5) MulitpartStream - gone from public to package visibility

   1) A maximum header size no longer exists. See FILEUPLOAD-108.
   2)+5) See
     http://marc.theaimsgroup.com/?l=jakarta-commons-dev&m=114988352104258&w=2
   4) This exception can no longer be thrown, because content-length=-1
       is allowed now.

All in all, do you really think that's as big an issue as you make it?


> One thing thats disappointing is that the last fileupload release had
> only 5 checkstyle issues. This one has 376 of which 200 are for tab
> characters which I personally dislike in source.

The reason is, IMO, that checkstyle is quite outdated. Examples:

- Why should I add a comment to a field called serialalversionuid?
- Why should I add a comment to an implementation or overwritten
  superclass method, when I know that the doclet will simply copy
  the interface or
- It's ridiculous to require comments for private fields, even if they are
  simply storage for getters and setters.
- What's the problem with trailing blanks? (I can understand your
  sentiment against tabs, btw.)

Please note, that I have enabled almost any warning that Eclipse
can trigger. And, believe me, these are more and more serious matters
than checkstyle detects. Nevertheless, I have eliminated almost all
warnings today, except those I cannot omit and I refuse to remove,
because I know better than checkstyle that it makes sense to keep
them. The code I have added contains *no* Eclipse warning. Elder
code does.


Jochen


--
My wife Mary and I have been married for forty-seven years and not
once have we had an argument serious enough to consider divorce;
murder, yes, but divorce, never.
(Jack Benny)

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: VOTE: Release commons-fileupload 1.2

Niall Pemberton
On 11/27/06, Jochen Wiedmann <[hidden email]> wrote:

> Hi, Niall,
>
> On 11/26/06, Niall Pemberton <[hidden email]> wrote:
>
> > I would have preferred the release be cut using maven1 rather than m2.
> > The maven1 build is tried and tested and the gripes of those of us
> > that checked out previous releases have been fixed in the maven1
> > build. I guess that doesn't matter if the release is up to scratch but
> > would be interested to know if others think we're ready for releases
> > using m2 yet?
>
> See for example
>
>     http://marc.theaimsgroup.com/?l=jakarta-commons-dev&m=115714503628112&w=2
>     http://marc.theaimsgroup.com/?t=115719054100004
>
> > The first issue I have with checking out this RC is that you've only
> > posted the "tar.gz" source and binary distros. I would have liked to
> > see the full set - zip versions and md5 checksums (the maven1 build
> > for fileupload creates the md5 checksums for you).
>
> Are you actually going to tell me, that this is an issue? (The archive
> type, not the checksum, which must of course, be added to
> distributables. As are .sha1 and .asc files, btw.) If so, changing it
> is as simple as adding one line to the assembly descriptor. But who do
> you believe is unable to use tar.gz in 2006?

I actually assumed you just hadn't uploaded them. But I'm glad now I
raised it since zip distros are artifacts the release should have
(whether you think they're a waste of time or not) - as the build
wasn't actually generating them then obviously it was a flaw which
needed fixing - which I'm glad you now have.

The real underlying issue here though is that I haven't seen you do a
release before - so I have nothing to give me confidence. The way to
starting building that is to at least produce all the artifacts for
the release. I'd have been happier if you'd followed what people
usually do - tagged the repository, cut the release candidate, made it
available for review and then called a vote.  You don't have to do any
of this, but you're more likely to get my vote if you do - and its
what I and quite a few others do:

   http://people.apache.org/~niallp/validator-1.3.1-rc1/
   http://people.apache.org/~rahul/commons/digester/
   http://people.apache.org/~bayard/commons-dbutils/1.1-RC1/

> > I think there are three serious issues with this RC:
> > 1) It doesn't comply with the new "ASF Source Header and Copyright
> > Notice Policy":
> >     http://www.apache.org/legal/src-headers.html
>
> I wasn't aware of a change in policy. You are right, that must be
> honoured. However, you already did it, so it cannot be counted as a
> reason for a -1, right?

Its a -1 for RC1 - now its fixed though should mean its not an issue
if you cut a RC2

> > 2) According to the jar's manifest file its been built using JDK
> > 1.6.0-rc. Even if JDK 1.6 had been (just) released using it for a
> > release would make me nervous - but using a RC version of Java to cut
> > the fileupload release is bad news IMO.
>
> Ok, I'll keep that in mind for the next approach.
>
>
> > 3) The clirr report you produced: which shows the following
> > incompatibilities with the previous fileupload version:
> >
> > 1) FileUploadBase - public constant MAX_HEADER_SIZE removed.
> > 2) FileUploadBase - protected method createItem removed
> > 3) FileUploadBase - two public constructors for
> > SizeLimitExceededException removed
> > 4) FileUploadBase - public static class UnknownSizeException removed
> > 5) MulitpartStream - gone from public to package visibility
>
>    1) A maximum header size no longer exists. See FILEUPLOAD-108.
>    2)+5) See
>      http://marc.theaimsgroup.com/?l=jakarta-commons-dev&m=114988352104258&w=2
>    4) This exception can no longer be thrown, because content-length=-1
>        is allowed now.
>
> All in all, do you really think that's as big an issue as you make it?

I think breaking backwards compatibility in a widely used library like
fileupload is a serious issue and as no-one else had reviewed your
release candidate I think highlighting it was important.  I agree the
issues don't look big - but in terms of compatibility one small change
could cause jar hell in a widely used library. Have you at least done
anything to verify whether any of these changes cause a problem with
any of the frameworks that use fileupload? I only went by the clirr
report and am not famailiar with why you removed these - but its
obvious that at least some of them (if not all) could have been
deprecated rather than removed. If there are good reasons why breaking
binary compatibility is preferable (or necessary) to deprecating then
IMO it would be good for you to state them and then it needs to be
decided whether putting out an incompatible release is acceptable or
the package name name should be changed:

http://www.mail-archive.com/commons-dev@.../msg85642.html

> > One thing thats disappointing is that the last fileupload release had
> > only 5 checkstyle issues. This one has 376 of which 200 are for tab
> > characters which I personally dislike in source.
>
> The reason is, IMO, that checkstyle is quite outdated. Examples:
>
> - Why should I add a comment to a field called serialalversionuid?
> - Why should I add a comment to an implementation or overwritten
>   superclass method, when I know that the doclet will simply copy
>   the interface or
> - It's ridiculous to require comments for private fields, even if they are
>   simply storage for getters and setters.
> - What's the problem with trailing blanks? (I can understand your
>   sentiment against tabs, btw.)
>
> Please note, that I have enabled almost any warning that Eclipse
> can trigger. And, believe me, these are more and more serious matters
> than checkstyle detects. Nevertheless, I have eliminated almost all
> warnings today, except those I cannot omit and I refuse to remove,
> because I know better than checkstyle that it makes sense to keep
> them. The code I have added contains *no* Eclipse warning. Elder
> code does.

I agree that checkstyle highlights things that are not an issue (e.g.
private fields) - mostly its just a case of changing the checkstyle
config to not emit the warnings for them. Having alot of trivial
issues reported hides the bigger ones though. The way commons works is
that it usually needs developers who are not working on a component to
vote to get a release out. While you may know there are no issues -
those of use not working on the code only have the reports to go by.
Having said that, AFAIK its not actually necessary to even have a
checkstyle report to do a release - but I'm happier if there is one
and that it is either clear or has very little on it. Thanks for
cleaning it up though :-)

Niall


> Jochen

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: VOTE: Release commons-fileupload 1.2

Martin Cooper-3
In reply to this post by jochen-2
On 11/26/06, Jochen Wiedmann <[hidden email]> wrote:

>
> Hi, Niall,
>
> On 11/26/06, Niall Pemberton <[hidden email]> wrote:
>
> > I would have preferred the release be cut using maven1 rather than m2.
> > The maven1 build is tried and tested and the gripes of those of us
> > that checked out previous releases have been fixed in the maven1
> > build. I guess that doesn't matter if the release is up to scratch but
> > would be interested to know if others think we're ready for releases
> > using m2 yet?
>
> See for example
>
>
> http://marc.theaimsgroup.com/?l=jakarta-commons-dev&m=115714503628112&w=2
>     http://marc.theaimsgroup.com/?t=115719054100004
>
>
>
> > The first issue I have with checking out this RC is that you've only
> > posted the "tar.gz" source and binary distros. I would have liked to
> > see the full set - zip versions and md5 checksums (the maven1 build
> > for fileupload creates the md5 checksums for you).
>
> Are you actually going to tell me, that this is an issue? (The archive
> type, not the checksum, which must of course, be added to
> distributables. As are .sha1 and .asc files, btw.) If so, changing it
> is as simple as adding one line to the assembly descriptor. But who do
> you believe is unable to use tar.gz in 2006?


All Commons components are released with both .zip and .tar.gz files. I
don't see a reason to change that now. And yes, I believe there are people
who either don't have tools to explode .tar.gz files or don't know that they
have such tools.

> I think there are three serious issues with this RC:
> > 1) It doesn't comply with the new "ASF Source Header and Copyright
> > Notice Policy":
> >     http://www.apache.org/legal/src-headers.html
>
> I wasn't aware of a change in policy. You are right, that must be
> honoured. However, you already did it, so it cannot be counted as a
> reason for a -1, right?


The current vote is on the distributables referenced in the VOTE message
that started this thread. Any changes made to the source tree since that
vote was started do _not_ affect this vote thread in any way. Once a new set
of distributables is created, and a new vote thread started, people can vote
differently from how they did on the current distributables, but vote
threads don't ever reference moving targets.

> 2) According to the jar's manifest file its been built using JDK
> > 1.6.0-rc. Even if JDK 1.6 had been (just) released using it for a
> > release would make me nervous - but using a RC version of Java to cut
> > the fileupload release is bad news IMO.
>
> Ok, I'll keep that in mind for the next approach.
>
>
>
> > 3) The clirr report you produced: which shows the following
> > incompatibilities with the previous fileupload version:
> >
> > 1) FileUploadBase - public constant MAX_HEADER_SIZE removed.
> > 2) FileUploadBase - protected method createItem removed
> > 3) FileUploadBase - two public constructors for
> > SizeLimitExceededException removed
> > 4) FileUploadBase - public static class UnknownSizeException removed
> > 5) MulitpartStream - gone from public to package visibility
>
>    1) A maximum header size no longer exists. See FILEUPLOAD-108.
>    2)+5) See
>
> http://marc.theaimsgroup.com/?l=jakarta-commons-dev&m=114988352104258&w=2
>    4) This exception can no longer be thrown, because content-length=-1
>        is allowed now.
>
> All in all, do you really think that's as big an issue as you make it?


Of course it is. These changes are incompatible in that some people's code
will no longer compile against the new version. The API contract has been
broken. Further, the changes were not preceded by deprecations, so people
who don't follow ongoing development will suddenly find their code broken,
rather than having had advance warning through deprecations.

Perhaps if this was a major version upgrade, this _might_ be acceptable, but
it's certainly not acceptable for a minor version upgrade, at least here at
Jakarta Commons.

> One thing thats disappointing is that the last fileupload release had
> > only 5 checkstyle issues. This one has 376 of which 200 are for tab
> > characters which I personally dislike in source.
>
> The reason is, IMO, that checkstyle is quite outdated. Examples:
>
> - Why should I add a comment to a field called serialalversionuid?
> - Why should I add a comment to an implementation or overwritten
>   superclass method, when I know that the doclet will simply copy
>   the interface or
> - It's ridiculous to require comments for private fields, even if they are
>   simply storage for getters and setters.
> - What's the problem with trailing blanks? (I can understand your
>   sentiment against tabs, btw.)
>
> Please note, that I have enabled almost any warning that Eclipse
> can trigger. And, believe me, these are more and more serious matters
> than checkstyle detects. Nevertheless, I have eliminated almost all
> warnings today, except those I cannot omit and I refuse to remove,
> because I know better than checkstyle that it makes sense to keep
> them. The code I have added contains *no* Eclipse warning. Elder
> code does.


I'm sorry, but I find your attitude quite disturbing here. You come in on a
project that has been very close to Checkstyle-clean for years, you make
changes that introduce hundreds of new violations, and you have the gall to
say that you refuse to fix the ones you don't agree with? That's not how we
work here at Commons either.

--
Martin Cooper


Jochen

>
>
> --
> My wife Mary and I have been married for forty-seven years and not
> once have we had an argument serious enough to consider divorce;
> murder, yes, but divorce, never.
> (Jack Benny)
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: VOTE: Release commons-fileupload 1.2

jochen-2
On 11/27/06, Martin Cooper <[hidden email]> wrote:

> Of course it is. These changes are incompatible in that some people's code
> will no longer compile against the new version.

In this particular case, the situation is like this: If people are
actually depending on the presence of this code, then they have, for
example, that this exception is thrown. An assumption, on which they
possibly depend, and which is no longer true. My personal style of
coding is that I would always prefer to be notified of such issues in
a very clear manner rather than having someone who ensures that I
believe everything is alright.

Besides, let me quote yourself:

    Multipartstream isn't (supposed to be) part of the public API

Sigh, I'll reintroduce this nonsense.



> I'm sorry, but I find your attitude quite disturbing here. You come in on a
> project that has been very close to Checkstyle-clean for years, you make
> changes that introduce hundreds of new violations, and you have the gall to
> say that you refuse to fix the ones you don't agree with? That's not how we
> work here at Commons either.

Yes, I share the feeling that this is not the style of work at
Commons. It rather seems to be having someone to pick up an almost
dead project, revitalize it with radical internal changes at the cost
of almost no public API change, give him no freedback without request
and even then almost no response to questions, suggestions and votes
(you being the almost sole exception), but whenever it comes to a
release, force him to do the important stuff like removing trailing
blanks, convert tabs into blanks, comment methods like
SomeInputStream.(close|read), and stuff like that.


Sorry, but I do indeed have gall in the sense of coming up from my waist.

Jochen


--
My wife Mary and I have been married for forty-seven years and not
once have we had an argument serious enough to consider divorce;
murder, yes, but divorce, never.
(Jack Benny)

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: VOTE: Release commons-fileupload 1.2

Henri Yandell
In reply to this post by jochen-2
On 11/26/06, Jochen Wiedmann <[hidden email]> wrote:

> Hi, Niall,
>
> On 11/26/06, Niall Pemberton <[hidden email]> wrote:
>
> > I would have preferred the release be cut using maven1 rather than m2.
> > The maven1 build is tried and tested and the gripes of those of us
> > that checked out previous releases have been fixed in the maven1
> > build. I guess that doesn't matter if the release is up to scratch but
> > would be interested to know if others think we're ready for releases
> > using m2 yet?
>
> See for example
>
>     http://marc.theaimsgroup.com/?l=jakarta-commons-dev&m=115714503628112&w=2
>     http://marc.theaimsgroup.com/?t=115719054100004

In those threads Phil points out that the Maven-2 sites need to look
the same as the current Commons sites.

There are a couple of sandbox components using Maven-2 sites as a test
- csv is one; but the css isn't right yet. Dennis (I think) talked
about needing a commons skin for Maven-2.

> > The first issue I have with checking out this RC is that you've only
> > posted the "tar.gz" source and binary distros. I would have liked to
> > see the full set - zip versions and md5 checksums (the maven1 build
> > for fileupload creates the md5 checksums for you).
>
> Are you actually going to tell me, that this is an issue? (The archive
> type, not the checksum, which must of course, be added to
> distributables. As are .sha1 and .asc files, btw.) If so, changing it
> is as simple as adding one line to the assembly descriptor. But who do
> you believe is unable to use tar.gz in 2006?

Windows XP can do zip and not tar.gz right? [without extra software].
And irritatingly, many Linux servers can do tar.gz and not zip.

Anyway, we produce both as the others have said.

> > 3) The clirr report you produced: which shows the following
> > incompatibilities with the previous fileupload version:
> >
> > 1) FileUploadBase - public constant MAX_HEADER_SIZE removed.
> > 2) FileUploadBase - protected method createItem removed
> > 3) FileUploadBase - two public constructors for
> > SizeLimitExceededException removed
> > 4) FileUploadBase - public static class UnknownSizeException removed
> > 5) MulitpartStream - gone from public to package visibility
>
>    1) A maximum header size no longer exists. See FILEUPLOAD-108.
>    2)+5) See
>      http://marc.theaimsgroup.com/?l=jakarta-commons-dev&m=114988352104258&w=2
>    4) This exception can no longer be thrown, because content-length=-1
>        is allowed now.
>
> All in all, do you really think that's as big an issue as you make it?

We try very hard nowadays to make sure our versioning is correct. A
minor release should have backwards compat - though I'm personally
happy with things being removed if their existence is considered a bug
(ie: 5).

Might be that we need to call this 2.0. In that case, we might want to
consider more API change - ie: removal of DefaultFileItem (?).

> > One thing thats disappointing is that the last fileupload release had
> > only 5 checkstyle issues. This one has 376 of which 200 are for tab
> > characters which I personally dislike in source.
>
> The reason is, IMO, that checkstyle is quite outdated. Examples:
>
> - Why should I add a comment to a field called serialalversionuid?
> - Why should I add a comment to an implementation or overwritten
>   superclass method, when I know that the doclet will simply copy
>   the interface or

This is a failing in Checkstyle I think. At this point the tail is
wagging the dog, so I'm +1 to ignoring these. Niall has a good point
that the noise of all the ignored Javadoc issues hides the important
things - but it's checkstyle not pmd/findbugs and you can still sift
through noise.

> - It's ridiculous to require comments for private fields, even if they are
>   simply storage for getters and setters.
> - What's the problem with trailing blanks? (I can understand your
>   sentiment against tabs, btw.)

Many +1s on the tabs. Trailing blanks are unnecessary, so no reason
not to remove them.

Hen

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: VOTE: Release commons-fileupload 1.2

Dennis Lundberg-2
Henri Yandell wrote:

> On 11/26/06, Jochen Wiedmann <[hidden email]> wrote:
>> Hi, Niall,
>>
>> On 11/26/06, Niall Pemberton <[hidden email]> wrote:
>>
>> > I would have preferred the release be cut using maven1 rather than m2.
>> > The maven1 build is tried and tested and the gripes of those of us
>> > that checked out previous releases have been fixed in the maven1
>> > build. I guess that doesn't matter if the release is up to scratch but
>> > would be interested to know if others think we're ready for releases
>> > using m2 yet?
>>
>> See for example
>>
>>    
>> http://marc.theaimsgroup.com/?l=jakarta-commons-dev&m=115714503628112&w=2
>>     http://marc.theaimsgroup.com/?t=115719054100004
>
> In those threads Phil points out that the Maven-2 sites need to look
> the same as the current Commons sites.
>
> There are a couple of sandbox components using Maven-2 sites as a test
> - csv is one; but the css isn't right yet. Dennis (I think) talked
> about needing a commons skin for Maven-2.

Yep, that's correct. The component sites generated by Maven 2 is not yet
up to par with the one's done by Maven 1. In my opinion more work is
needed before we can use Maven 2 to generate and publish our sites.
Setting up a commons-skin is the "right way" to go, but I have so far
been unsuccessful in creating one.

<snip/>


--
Dennis Lundberg

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]