commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: [VOTE] Release of commons-email-1.3 based on RC5
Date Wed, 12 Dec 2012 20:42:35 GMT
On Wed, Dec 12, 2012 at 3:31 PM, Thomas Neidhart
<thomas.neidhart@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 <garydgregory@gmail.com> wrote:
> >>> On Wed, Dec 12, 2012 at 3:59 AM, Thomas Neidhart
> >>> <thomas.neidhart@gmail.com>wrote:
> >>>
> >>>> On Wed, Dec 12, 2012 at 4:58 AM, Gary Gregory <garydgregory@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.neidhart@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-unsubscribe@commons.apache.org
> >>>>>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> E-Mail: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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-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
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message