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-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
