[VOTE] Release Imaging 1.0 from RC4

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

[VOTE] Release Imaging 1.0 from RC4

Damjan Jovanovic
Please vote on releasing commons-imaging 1.0 from RC4.

A number of things were changed from RC2: dead local store warnings in
Findbugs were fixed, missing ASL headers were added to Javadoc files,
developers and contributors were populated into pom.xml, a few
last-minute bugs were also fixed, and the build was deployed to Nexus
this time. But I am unsure whether the SCM is set correctly in pom.xml
- should it be tags/IMAGING_1_0_RC4 for a release? Also is
commons.rc.version correct?

Tag:
https://svn.apache.org/repos/asf/commons/proper/imaging/tags/IMAGING_1_0_RC4

Site:
http://people.apache.org/~damjan/imaging-1.0rc4/

Binaries:
https://repository.apache.org/content/repositories/staging/org/apache/commons/commons-imaging/

Votes, please.  This vote will close in 72 hours, Friday 28 September 2012
at 04:33 GMT.

[ ] +1 Release these artifacts
[ ] +0 OK, but...
[ ] -0 OK, but really should fix...
[ ] -1 I oppose this release because...

Thank you!

Damjan Jovanovic

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

Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Imaging 1.0 from RC4

garydgregory
Hi All,

I happy to see another RC come along! :) This is large code base so I
appreciate that the reports generate a lot of potential work.

-1: Unapproved license:

src/main/java/org/apache/commons/imaging/formats/jpeg/segments/App14Segment.java

Fix obvious FindBugs (to me only?):

- Possible NPE with
NP_IMMEDIATE_DEREFERENCE_OF_READLINE<http://findbugs.sourceforge.net/bugDescriptions.html#NP_IMMEDIATE_DEREFERENCE_OF_READLINE>@
https://people.apache.org/~damjan/imaging-1.0rc4/xref/org/apache/commons/imaging/formats/rgbe/RgbeInfo.html#103
- Is this useless or an incomplete impl?: Useless control flow in
org.apache.commons.imaging.palette.PaletteFactory.makePaletteFancy(BufferedImage)
STYLE UCF_USELESS_CONTROL_FLOW<http://findbugs.sourceforge.net/bugDescriptions.html#UCF_USELESS_CONTROL_FLOW>
54<https://people.apache.org/%7Edamjan/imaging-1.0rc4/xref/org/apache/commons/imaging/palette/PaletteFactory.html#54>
-
If we want non-standard class name (starts with a lower case, then Javadoc
would be nice to understand why this is justified:
      public static class tEXt extends PngText
      public static class zTXt extends PngText
      public static class iTXt extends PngText
- What about the
RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE<http://findbugs.sourceforge.net/bugDescriptions.html#RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE>?
- What about the
FE_FLOATING_POINT_EQUALITY<http://findbugs.sourceforge.net/bugDescriptions.html#FE_FLOATING_POINT_EQUALITY>?
- IIRC, the EI_EXPOSE_REP<http://findbugs.sourceforge.net/bugDescriptions.html#EI_EXPOSE_REP>issues
are just a normal by-product of the design? Please confirm.

- PMD
  - do the easy clean ups of "Unnecessary final modifier in final class".
  - "Avoid empty catch blocks": Add a comment as to why the block is empty,
for example "This exception cannot be thrown because...". This is important
for anyone who tries to grok the code.
  - "Avoid unused method/fields/etc": Is this unimplemented code or cruft?
If cruft, it should be removed before we give ourselves unneeded BC
headaches.

- CDP: lots of those but I do not know the code base enough to tell how
easy it would be to refactor duplications and if refactoring is justified.

- Code coverage: overall, is not great.

These packages have 0%.code coverage.
  - org.apache.commons.imaging.formats.transparencyfilters
  - org.apache.commons.imaging.formats.psd.dataparsers
  - org.apache.commons.imaging.formats.psd.datareaders
  - org.apache.commons.imaging.icc

Oddly org.apache.commons.imaging.util has only 17% which I would think
would be higher since it is a util package.

- Process

This might be a case of RM process but when I RM I set the release date in
changes.xml to the date I cut the RC. If the RC passes, then you are done,
otherwise you edit it again for the next RC.

- Javadoc

Only 3 out of ~40 packages have comments, which makes it harder to find
your way around.

- Checkstyle is not strict enough
  - IMO should always complain about missing { } in blocks.

There also a lot of generics warnings (in Eclipse, any IDE will tell you
these days.)

Sorry to make it a long laundry list, but there is only one 1.0 ;)

There is probably more of course, but I have to go now...

Gary


On Tue, Sep 25, 2012 at 12:33 AM, Damjan Jovanovic <[hidden email]>wrote:

> Please vote on releasing commons-imaging 1.0 from RC4.
>
> A number of things were changed from RC2: dead local store warnings in
> Findbugs were fixed, missing ASL headers were added to Javadoc files,
> developers and contributors were populated into pom.xml, a few
> last-minute bugs were also fixed, and the build was deployed to Nexus
> this time. But I am unsure whether the SCM is set correctly in pom.xml
> - should it be tags/IMAGING_1_0_RC4 for a release? Also is
> commons.rc.version correct?
>
> Tag:
>
> https://svn.apache.org/repos/asf/commons/proper/imaging/tags/IMAGING_1_0_RC4
>
> Site:
> http://people.apache.org/~damjan/imaging-1.0rc4/
>
> Binaries:
>
> https://repository.apache.org/content/repositories/staging/org/apache/commons/commons-imaging/
>
> Votes, please.  This vote will close in 72 hours, Friday 28 September 2012
> at 04:33 GMT.
>
> [ ] +1 Release these artifacts
> [ ] +0 OK, but...
> [ ] -0 OK, but really should fix...
> [ ] -1 I oppose this release because...
>
> Thank you!
>
> Damjan Jovanovic
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>


--
E-Mail: [hidden email] | [hidden email]
JUnit in Action, 2nd Ed: <http://goog_1249600977>http://bit.ly/ECvg0
Spring Batch in Action: <http://s.apache.org/HOq>http://bit.ly/bqpbCK
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Imaging 1.0 from RC4

Damjan Jovanovic
On Tue, Sep 25, 2012 at 3:22 PM, Gary Gregory <[hidden email]> wrote:
> Hi All,
>
> I happy to see another RC come along! :) This is large code base so I
> appreciate that the reports generate a lot of potential work.

I planned to send this email out just over a year ago, shortly after
yours, but since we might actually get to release this time let me
send it out now for reference.

> -1: Unapproved license:
>
> src/main/java/org/apache/commons/imaging/formats/jpeg/segments/App14Segment.java

Fixed.

> Fix obvious FindBugs (to me only?):
>
> - Possible NPE with
> NP_IMMEDIATE_DEREFERENCE_OF_READLINE<http://findbugs.sourceforge.net/bugDescriptions.html#NP_IMMEDIATE_DEREFERENCE_OF_READLINE>@
> https://people.apache.org/~damjan/imaging-1.0rc4/xref/org/apache/commons/imaging/formats/rgbe/RgbeInfo.html#103

Findbugs false positive: it believes every method named "readLine()"
behaves like BufferedReader's. I've renamed it to "readNextLine()".

> - Is this useless or an incomplete impl?: Useless control flow in
> org.apache.commons.imaging.palette.PaletteFactory.makePaletteFancy(BufferedImage)
> STYLE UCF_USELESS_CONTROL_FLOW<http://findbugs.sourceforge.net/bugDescriptions.html#UCF_USELESS_CONTROL_FLOW>
> 54<https://people.apache.org/%7Edamjan/imaging-1.0rc4/xref/org/apache/commons/imaging/palette/PaletteFactory.html#54>

Incomplete impl, fixed now.

> -
> If we want non-standard class name (starts with a lower case, then Javadoc
> would be nice to understand why this is justified:
>       public static class tEXt extends PngText
>       public static class zTXt extends PngText
>       public static class iTXt extends PngText

Because that's the case used for PNG field markers. Converted to
standard names anyway.

> - What about the
> RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE<http://findbugs.sourceforge.net/bugDescriptions.html#RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE>?

The only reason that value is not null, is because the method
providing it hasn't been overridden - and that method is neither
private nor final so it can be overridden. Fixed anyway.

> - What about the
> FE_FLOATING_POINT_EQUALITY<http://findbugs.sourceforge.net/bugDescriptions.html#FE_FLOATING_POINT_EQUALITY>?

Probably a false positive (comparison of variables to the maximum
amongst themselves) but I worked around it.

> - IIRC, the EI_EXPOSE_REP<http://findbugs.sourceforge.net/bugDescriptions.html#EI_EXPOSE_REP>issues
> are just a normal by-product of the design? Please confirm.

Yes they are.

> - PMD
>   - do the easy clean ups of "Unnecessary final modifier in final class".

Done.

>   - "Avoid empty catch blocks": Add a comment as to why the block is empty,
> for example "This exception cannot be thrown because...". This is important
> for anyone who tries to grok the code.

The name I give to the exception variable (ignore, cannotHappen)
should make it obvious.

>   - "Avoid unused method/fields/etc": Is this unimplemented code or cruft?
> If cruft, it should be removed before we give ourselves unneeded BC
> headaches.
>
> - CDP: lots of those but I do not know the code base enough to tell how
> easy it would be to refactor duplications and if refactoring is justified.
>
> - Code coverage: overall, is not great.
>
> These packages have 0%.code coverage.
>   - org.apache.commons.imaging.formats.transparencyfilters
>   - org.apache.commons.imaging.formats.psd.dataparsers
>   - org.apache.commons.imaging.formats.psd.datareaders
>   - org.apache.commons.imaging.icc
>
> Oddly org.apache.commons.imaging.util has only 17% which I would think
> would be higher since it is a util package.

Even to get that low test coverage, it already takes 40+ minutes to
run "mvn site" due to Corbertura. I've added some tests so it's > 0
now. The real "util" package is org.apache.commons.imaging.common.

> - Process
>
> This might be a case of RM process but when I RM I set the release date in
> changes.xml to the date I cut the RC. If the RC passes, then you are done,
> otherwise you edit it again for the next RC.

Ok thank you.

> - Javadoc
>
> Only 3 out of ~40 packages have comments, which makes it harder to find
> your way around.

Added a few more package comments, but the API is largely designed to
be used from the org.apache.commons.imaging.Imaging class alone.

> - Checkstyle is not strict enough
>   - IMO should always complain about missing { } in blocks.

I thought there was no official coding style for commons? But I've
added it anyway.

> There also a lot of generics warnings (in Eclipse, any IDE will tell you
> these days.)

Upgrades from Java 1.4 are never fun. These were largely fixed in
code, but tests might have a few.

> Sorry to make it a long laundry list, but there is only one 1.0 ;)

You should see what was already fixed :).

> There is probably more of course, but I have to go now...
>
> Gary

Thank you very much
Damjan

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

Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Imaging 1.0 from RC4

Andreas Lehmkuehler
Hi,

Am 21.10.2013 16:25, schrieb Damjan Jovanovic:

> On Tue, Sep 25, 2012 at 3:22 PM, Gary Gregory <[hidden email]> wrote:
>> Hi All,
>>
>> I happy to see another RC come along! :) This is large code base so I
>> appreciate that the reports generate a lot of potential work.
>
> I planned to send this email out just over a year ago, shortly after
> yours, but since we might actually get to release this time let me
> send it out now for reference.
>
> SNIP
I ran a dry maven test (mvn release:prepare -DdryRun=true) and everything looks
good. But I got a lot of redundant artifacts


commons-imaging-1.0-SNAPSHOT.jar
commons-imaging-1.0-SNAPSHOT-bin.zip
commons-imaging-1.0-SNAPSHOT-bin.tar.gz

commons-imaging-1.0-SNAPSHOT-source-release.zip
commons-imaging-1.0-SNAPSHOT-sources.jar
commons-imaging-1.0-SNAPSHOT-src.zip
commons-imaging-1.0-SNAPSHOT-src.tar.gz

I guess those are not created with intent, are they?

> Thank you very much
> Damjan
Thanks for the ongoing effort to release/work on imaging

BR
Andreas Lehmkühler


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

Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Imaging 1.0 from RC4

sebb-2-2
On 21 October 2013 18:27, Andreas Lehmkuehler <[hidden email]> wrote:

> Hi,
>
> Am 21.10.2013 16:25, schrieb Damjan Jovanovic:
>>
>> On Tue, Sep 25, 2012 at 3:22 PM, Gary Gregory <[hidden email]>
>> wrote:
>>>
>>> Hi All,
>>>
>>> I happy to see another RC come along! :) This is large code base so I
>>> appreciate that the reports generate a lot of potential work.
>>
>>
>> I planned to send this email out just over a year ago, shortly after
>> yours, but since we might actually get to release this time let me
>> send it out now for reference.
>>
>> SNIP
>
> I ran a dry maven test (mvn release:prepare -DdryRun=true) and everything
> looks good. But I got a lot of redundant artifacts
>
>
> commons-imaging-1.0-SNAPSHOT.jar
> commons-imaging-1.0-SNAPSHOT-bin.zip
> commons-imaging-1.0-SNAPSHOT-bin.tar.gz
>
> commons-imaging-1.0-SNAPSHOT-source-release.zip
> commons-imaging-1.0-SNAPSHOT-sources.jar
> commons-imaging-1.0-SNAPSHOT-src.zip
> commons-imaging-1.0-SNAPSHOT-src.tar.gz
>
> I guess those are not created with intent, are they?

The bin and src zip and targz archives _are_ needed; these are
released to the ASF mirrors.
[That they are attached to the project and end up in Nexus staging is
a long story.]

source-release.zip may be spurious.

jar and sources.jar look like standard Maven jars.

>
>> Thank you very much
>> Damjan
>
> Thanks for the ongoing effort to release/work on imaging
>
> BR
> Andreas Lehmkühler
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

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

Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Imaging 1.0 from RC4

Andreas Lehmkuehler
Hi,

Am 21.10.2013 19:33, schrieb sebb:

> On 21 October 2013 18:27, Andreas Lehmkuehler <[hidden email]> wrote:
>> Hi,
>>
>> Am 21.10.2013 16:25, schrieb Damjan Jovanovic:
>>>
>>> On Tue, Sep 25, 2012 at 3:22 PM, Gary Gregory <[hidden email]>
>>> wrote:
>>>>
>>>> Hi All,
>>>>
>>>> I happy to see another RC come along! :) This is large code base so I
>>>> appreciate that the reports generate a lot of potential work.
>>>
>>>
>>> I planned to send this email out just over a year ago, shortly after
>>> yours, but since we might actually get to release this time let me
>>> send it out now for reference.
>>>
>>> SNIP
>>
>> I ran a dry maven test (mvn release:prepare -DdryRun=true) and everything
>> looks good. But I got a lot of redundant artifacts
>>
>>
>> commons-imaging-1.0-SNAPSHOT.jar
>> commons-imaging-1.0-SNAPSHOT-bin.zip
>> commons-imaging-1.0-SNAPSHOT-bin.tar.gz
>>
>> commons-imaging-1.0-SNAPSHOT-source-release.zip
>> commons-imaging-1.0-SNAPSHOT-sources.jar
>> commons-imaging-1.0-SNAPSHOT-src.zip
>> commons-imaging-1.0-SNAPSHOT-src.tar.gz
>>
>> I guess those are not created with intent, are they?
>
> The bin and src zip and targz archives _are_ needed; these are
> released to the ASF mirrors.
> [That they are attached to the project and end up in Nexus staging is
> a long story.]
Is this a commons policy? PDFBox [1] has one source archive and a couple
of precompiled jars (one for each component and 2 bundling all neede components
for 2 typical usecases).

> source-release.zip may be spurious.
>
> jar and sources.jar look like standard Maven jars.
>
>>
>>> Thank you very much
>>> Damjan
>>
>> Thanks for the ongoing effort to release/work on imaging
>>
>> BR
>> Andreas Lehmkühler

BR
Andreas Lehmkühler
[1] http://www.apache.org/dyn/closer.cgi/pdfbox/1.8.2


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

Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Imaging 1.0 from RC4

sebb-2-2
On 21 October 2013 18:49, Andreas Lehmkuehler <[hidden email]> wrote:

> Hi,
>
> Am 21.10.2013 19:33, schrieb sebb:
>
>> On 21 October 2013 18:27, Andreas Lehmkuehler <[hidden email]> wrote:
>>>
>>> Hi,
>>>
>>> Am 21.10.2013 16:25, schrieb Damjan Jovanovic:
>>>>
>>>>
>>>> On Tue, Sep 25, 2012 at 3:22 PM, Gary Gregory <[hidden email]>
>>>> wrote:
>>>>>
>>>>>
>>>>> Hi All,
>>>>>
>>>>> I happy to see another RC come along! :) This is large code base so I
>>>>> appreciate that the reports generate a lot of potential work.
>>>>
>>>>
>>>>
>>>> I planned to send this email out just over a year ago, shortly after
>>>> yours, but since we might actually get to release this time let me
>>>> send it out now for reference.
>>>>
>>>> SNIP
>>>
>>>
>>> I ran a dry maven test (mvn release:prepare -DdryRun=true) and everything
>>> looks good. But I got a lot of redundant artifacts
>>>
>>>
>>> commons-imaging-1.0-SNAPSHOT.jar
>>> commons-imaging-1.0-SNAPSHOT-bin.zip
>>> commons-imaging-1.0-SNAPSHOT-bin.tar.gz
>>>
>>> commons-imaging-1.0-SNAPSHOT-source-release.zip
>>> commons-imaging-1.0-SNAPSHOT-sources.jar
>>> commons-imaging-1.0-SNAPSHOT-src.zip
>>> commons-imaging-1.0-SNAPSHOT-src.tar.gz
>>>
>>> I guess those are not created with intent, are they?
>>
>>
>> The bin and src zip and targz archives _are_ needed; these are
>> released to the ASF mirrors.
>> [That they are attached to the project and end up in Nexus staging is
>> a long story.]
>
> Is this a commons policy?

The duplicate zip and tar archives are common to all commons components (AFAIK).
I think the reason for this is that not all Windows systems support
tar archives, and until recently many Unix systems did not support
Zip; also zip is bigger.

Maven releases include binary jars (obviously) as well as source and
javadocs and test jars.
The source and javadoc jars are useful for IDE debugging.
The test jar is useful for compatibility testing.

Have a look at the other components under

http://www.apache.org/dist/commons/
and
http://repo1.maven.org/maven2/org/apache/commons/

I think you'll find quite a lot of other projects follow much the same strategy.

> PDFBox [1] has one source archive and a couple
> of precompiled jars (one for each component and 2 bundling all neede
> components
> for 2 typical usecases).
>
>
>> source-release.zip may be spurious.
>>
>> jar and sources.jar look like standard Maven jars.
>>
>>>
>>>> Thank you very much
>>>> Damjan
>>>
>>>
>>> Thanks for the ongoing effort to release/work on imaging
>>>
>>> BR
>>> Andreas Lehmkühler
>
>
> BR
> Andreas Lehmkühler
> [1] http://www.apache.org/dyn/closer.cgi/pdfbox/1.8.2
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

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

Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Imaging 1.0 from RC4

Henning Schmiedehausen-2
In reply to this post by sebb-2-2
On Mon, Oct 21, 2013 at 7:33 AM, sebb <[hidden email]> wrote:

[...]

source-release.zip may be spurious.
>

These files get created when running

mvn -DdryRun release:prepare

instead of

mvn -Prelease -DdryRun release:prepare

The first one picks up the  sourceReleaseAssemblyDescriptor from the apache
parent.

("spurious" is expert speak for "I have no clue". :-) )

-h
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Imaging 1.0 from RC4

sebb-2-2
On 22 October 2013 07:50, Henning Schmiedehausen
<[hidden email]> wrote:

> On Mon, Oct 21, 2013 at 7:33 AM, sebb <[hidden email]> wrote:
>
> [...]
>
> source-release.zip may be spurious.
>>
>
> These files get created when running
>
> mvn -DdryRun release:prepare
>
> instead of
>
> mvn -Prelease -DdryRun release:prepare
>
> The first one picks up the  sourceReleaseAssemblyDescriptor from the apache
> parent.

Ah yes, the Apache parent pom has a release profile, but it does not
quite do what Commons needs
That's why the CP release profile was created.

> ("spurious" is expert speak for "I have no clue". :-) )
>
> -h

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

Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Imaging 1.0 from RC4

Bernd Eckenfels
Am 22.10.2013, 13:20 Uhr, schrieb sebb <[hidden email]>:
> Ah yes, the Apache parent pom has a release profile, but it does not
> quite do what Commons needs
> That's why the CP release profile was created.

There is a "useReleaseProfile" property which you can set to "false" in  
the pom/configuration to make sure it will not activate "release-profile"  
and there is a releaseProfile= list of the profiles you want to enforce.  
Using both in the POM makes the build more self contained.

Gruss
Bernd
--
http://www.zusammenkunft.net

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