On Wed, Dec 12, 2012 at 3:31 PM, Thomas Neidhart <thomas.neidh...@gmail.com>wrote:
> On 12/12/2012 08:39 PM, Siegfried Goeschl 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 > > > > * 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 > > Hi Siegfried, > > thanks for your feedback. > > Taking this into account, together with the posts from sebb and Gary, > imho the best solutions is as sebb proposed: > > * Change EmailConstants to a public final class > * Add back the constants to the Email class by referencing them from > EmailConstants > * Mark the constants in Email as deprecated > +1 to these three changes. > > btw. there are several constants that have not been in use since the 1.0 > release: > > String SENDER_EMAIL = "sender.email"; > String SENDER_NAME = "sender.name"; > String RECEIVER_EMAIL = "receiver.email"; > String RECEIVER_NAME = "receiver.name"; > String EMAIL_SUBJECT = "email.subject"; > String EMAIL_BODY = "email.body"; > String CONTENT_TYPE = "content.type"; > String ATTACHMENTS = "attachments"; > String FILE_SERVER = "file.server"; > > Does anybody know how or if they are in use? They are now marked as > deprecated, but I am really curious about there origin. > You could add "not in use since..." to the deprecated message. Gary > > Does everybody agree on wha? > > Thomas > > > > > 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 > > -- 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
