commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Siegfried Goeschl <sgoes...@gmx.at>
Subject Re: [VOTE][CANCEL] The vote for commons-email-1.3 based on RC2 in cancelled
Date Sun, 11 Dec 2011 22:42:49 GMT
Hi folks,

I ran the commons-email-1.2 test suite with commons-email-1.3 and got

[junit] Running org.apache.commons.mail.EmailTest
[junit] Testsuite: org.apache.commons.mail.EmailTest
[junit] Tests run: 39, Failures: 0, Errors: 17, Time elapsed: 0.252 sec
[junit] Tests run: 39, Failures: 0, Errors: 17, Time elapsed: 0.252 sec

[junit] Testcase: testGetSetSession took 0.012 sec
[junit] 	Caused an ERROR
[junit] 
org.apache.commons.mail.mocks.MockEmailConcrete.setMailSession(Ljavax/mail/Session;)V
[junit] java.lang.NoSuchMethodError: 
org.apache.commons.mail.mocks.MockEmailConcrete.setMailSession(Ljavax/mail/Session;)V
[junit] 	at 
org.apache.commons.mail.EmailTest.testGetSetSession(EmailTest.java:108)

Well, had another run with some other code getting

ests run: 11, Failures: 0, Errors: 10, Skipped: 0, Time elapsed: 0.451 
sec <<< FAILURE!
testDefaultDomain(org.apache.fulcrum.commonsemail.CommonsEmailServiceTest) 
  Time elapsed: 0.147 sec  <<< ERROR!
java.lang.NoSuchMethodError: 
org.apache.commons.mail.Email.setAuthentication(Ljava/lang/String;Ljava/lang/String;)V
	at 
org.apache.fulcrum.commonsemail.impl.CommonsEmailServiceImpl.configure(CommonsEmailServiceImpl.java:1007)
	at 
org.apache.fulcrum.commonsemail.impl.CommonsEmailServiceImpl.createSimpleEmail(CommonsEmailServiceImpl.java:285)
	at 
org.apache.fulcrum.commonsemail.CommonsEmailServiceTest.testDefaultDomain(CommonsEmailServiceTest.java:274)


So Sebastian was right ...

+) changing the return type from "void" to something else breaks binary 
compatibility

+) moving the constants from Email to EmailConstants.java was had no impact


Sebastian, kudos given ... :-)


Cheers,

Siegfried Goeschl



On 11.12.11 22:06, Siegfried Goeschl wrote:
> Hi folks,
>
> reviewing the release candidate showed a few problems/discussion points
>
> 1) Moving constant from Email.java to EmailConstants,java
> ==================================================
>
> I made the following change
>
> +) adding EmailConstants
> +) Email implements EmailConstants
>
> public interface EmailConstants {}
> public abstract class Email implements EmailConstants {}
>
> We have different opinions out there
>
> +) Gary - in general unhappy about an interface containing constants
> only, and see issues with source code and binary compatibility
> +) Sebastian - sees no issues with binary compatibility
> +) I personally don't see any issues otherwise I would not have made the
> change
>
>
> 2) Renaming of protected fields
> ==================================================
>
> The code exposes every field as protected which makes me unhappy since
> the same filed could be accessed using a getter/setter as well. Due to
> EMAIL-105 (https://issues.apache.org/jira/browse/EMAIL-105) I renamed
> two protected fields on order to clarify the behavior
>
> * tls ==> startTlsEnabled
> * ssl ==> sslOnConnect
>
> The original getters/setters are still there but deprecated now
>
> +) Sebastian pointed out that this breaks binary compatibility
> +) I think along the lines that the protected fields should not be used
> at all if there is a getter/setter available
>
> The question - does this change requires a new major release?
>
>
> 3) Return type of setters changed from "void" to "this"
> ==================================================
>
> I changed the return type of setters to support something like this
>
> email.setMailSession()setDebug().setContent();
>
> instead of
>
> email.setMailSession();
> email.setDebug();
> email.setContent();
>
> +) Sebastian pointed out that this breaks binary compatibility (a
> similar issues happened in commons-io)
> +) based on my knowledge I doubt that but have to admit that Sebastian
> knows a lot of things better than I do ... :-)
>
> I thing the way to go is to run the commons-email-1.2 test suite with
> commons-email-1.3 and to report the result
>
> 4) The source zip is missing
> ==================================================
>
> No discussions about that ... :-)
>
>
> 5) OS-dependency in DataSourceFileResolverTest.testResolvingFileLenient
> ==================================================
>
> No discussions about that ... :-)
>
>
> 6) RAT Complaints
> ==================================================
>
> The "mime.type" and four test email messages have no ASL - with the
> newest commons-parent it should be possible to exclude files from RAT
>
>
> I'm sort of stuck here - IMHO the changes regarding 1), 2) and 3) are
> not big enough to justify a new major release whereas the library has
> enough rough edges to look forward an clean-up and major release. But
> for doing that I simple have not enough time for the next weeks ... any
> opinions out there?
>
> Cheers,
>
> Siegfried Goeschl
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message