On 12 December 2012 19:39, Siegfried Goeschl <sgoes...@gmx.at> wrote: > Hi folks, > > to avoid the full circles here > > * I hopefully fixed the binary compatibility issue, wrote test and also > tested it with my production code > > * I moved the constants to EmailConstants because there are many constants > and using an non-trivial implementation class (Email) to access a few > constants is also questionable in my opinion
A better solution might have been to use a constant class rather than an interface. The Email class can then reference the constant class: public static final String XYZ = EmailConstants.XYZ; Other classes can then reference EmailConstants directly. > * The design of commons-email is flawed in a few ways and there is not a lot > you can fix in a major version. So I suggest that we focus on delivered > value instead > > Cheers, > > Siegfried Goeschl > > > > On 12.12.12 15:06, sebb wrote: >> >> On 12 December 2012 13:17, Gary Gregory <garydgreg...@gmail.com> wrote: >>> >>> On Wed, Dec 12, 2012 at 3:59 AM, Thomas Neidhart >>> <thomas.neidh...@gmail.com>wrote: >>> >>>> On Wed, Dec 12, 2012 at 4:58 AM, Gary Gregory <garydgreg...@gmail.com >>>>> >>>>> wrote: >>>> >>>> >>>>> Thank you for doing another RC. >>>>> >>>>> While I was digging for a justification of the Clirr errors, I found >>>>> this >>>>> in the release notes: "Clirr reports several errors for this release >>>>> due >>>> >>>> to >>>>> >>>>> moving constants from the Email class to the newly introduced >>>>> EmailConstants interface. These changes are guaranteed to be binary >>>>> compatible." >>>>> >>>>> Is it really binary compatible? What if I use reflection to access the >>>>> constant on Email, will the reflection call be redirected to >>>>> EmailConstants? There's unit test for ya ;) >>>>> >>>>> Using an interface to define constants is a no-no in my book. I've seen >>>>> this discussed before in other places and for a long time, but to >>>>> summarize, I see an interface as defining a contract for a class to >>>>> implement. A constant does not fit. >>>>> >>>>> Constants in interface feels like a hack to provide the short hand of a >>>>> class implementing an interface just to be able to access the constants >>>>> without qualifying them with a type. Not nice design IMO and a dubious >>>>> us >>>>> of an interface, very Java 1.0. It seems that static imports is another >>>>> attempt to solve this desire for a short hand to use constants. >>>>> >>>>> What to do? Move the constants back to their 1.2? What's so bad about >>>> >>>> that? >>>>> >>>>> Hm... >>>>> >>>>> Make the EmailConstants a class instead of an interface? If binary >>>>> compatible is broken, the constants have to move back, and you can >>>>> still >>>>> have a new EmailConstants class and deprecate the old constants to >>>>> point >>>> >>>> to >>>>> >>>>> the new class. >>>>> >>>>> Maybe I'll see this more clearly in the AM... >>>>> >>>>> Interested in you all's feedback. >>>>> >>>> >>>> Hi Gary, >>>> >>>> well, I think we go in circles with this change ;-). >>>> >>>> I assumed that this topic is settled after reading the comment from sebb >>>> in >>>> the RC2 thread (see http://markmail.org/message/svrb7nf3ocz7lgmd). >>>> >>>> Otoh, it's the first time I see constants in an interface and would be >>>> in >>>> favor of reverting to the previous version (also because I do not fully >>>> understand the rationale behind the change, some of the constants are >>>> not >>>> even used and thus have been deprecated). >>>> >>> -- >>> >>>> >>>> Maybe we should postpone this kind of refactoring to 2.0 and do it then >>>> in >>>> a proper way. Introducing this interface just created headaches and I >>>> also >>>> had to disable some checks (e.g. InterfaceIsAType in checkstyle) because >>>> of >>>> it. >>>> >>> >>> That sounds like a good way to go to get 1.3 out the door. >> >> >> Agreed. >> >>> Gary >>> >>> >>>> >>>> Thomas >>>> >>>> >>>>> On Tue, Dec 11, 2012 at 5:24 PM, Thomas Neidhart >>>>> <thomas.neidh...@gmail.com>wrote: >>>>> >>>>>> Hi, >>>>>> >>>>>> I would like to call a vote from commons-email-1.3 based on RC5. >>>>>> >>>>>> This release candidate has the following changes compared to RC4 >>>>>> >>>>>> +) update index and building page with correct information wrt Java >>>>>> compatibility >>>>>> +) update release notes with info on Java compatibility and Clirr >>>> >>>> errors >>>>>> >>>>>> +) fix svn:keywords for all source files and remove use of $Date$ tags >>>>>> +) add $Id$ tags for all newly introduced source files in 1.3 >>>>>> +) update javax.mail.mail dependency to 1.4.5 >>>>>> +) fix PMD warnings and add NOPMD comment for false positives >>>>>> +) added findbugs exclude filter for false positives >>>>>> +) fix release date in changes.xml >>>>>> +) correctly removed *.asc.[md5,sha1] files from Nexus staging area >>>>>> >>>>>> The files: >>>>>> >>>>>> The artifacts are deployed to Nexus: >>>>>> >>>> https://repository.apache.org/content/repositories/orgapachecommons-137/ >>>>>> >>>>>> >>>>>> The tag: >>>>>> >>>>> >>>> >>>> https://svn.apache.org/repos/asf/commons/proper/email/tags/EMAIL_1_3_RC5/ >>>>>> >>>>>> >>>>>> The site: >>>>>> http://people.apache.org/builds/commons/email/1.3/RC5/ >>>>>> >>>>>> Additional Notes: >>>>>> >>>>>> o the download page and api links to older releases only work on >>>>>> the published site and will be corrected after release. >>>>>> >>>>>> Please take a look at the commons-email-1.3 artifacts and vote! >>>>>> >>>>>> ------------------------------------------------ >>>>>> [ ] +1 release it >>>>>> [ ] +0 go ahead I don't care >>>>>> [ ] -1 no, do not release it because >>>>>> ------------------------------------------------ >>>>>> >>>>>> Vote will remain open for at least 72 hours. >>>>>> >>>>>> Thanks in advance, >>>>>> >>>>>> Thomas >>>>>> >>>>>> --------------------------------------------------------------------- >>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org >>>>> 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 >>>>> >>>> >>> >>> >>> >>> -- >>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org >>> 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 >> >> >> --------------------------------------------------------------------- >> 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 > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
