geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vamsavardhana Reddy" <c1vams...@gmail.com>
Subject Re: svn commit: r479586 - /geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Date Tue, 28 Nov 2006 06:08:20 GMT
By copy+paste, I did not mean copying the JIRA content and pasting it into
the change log.  I meant to say, "anyway I am including the summary of
change in JIRA comment while resolving the JIRA.  I will copy+paste the same
into svn change log."

--vamsi

On 11/28/06, Jason Dillon <jason@planet57.com> wrote:
>
> I don't think it is appropriate to simply copy/paste the jira content into
> the svn change comment.  I expect the jira issue to contain a lot more
> details and some comments about the change, where I expect the svn change
> comment to briefly explain the change.
> --jason
>
>
> On Nov 27, 2006, at 9:57 PM, Vamsavardhana Reddy wrote:
>
> Hi Jason,
>
> Sometime ago I had difficulty in figuring out what revision(s) fixed a
> particular JIRA and vice versa.  So, I have made it a practice to include
> the JIRA number in the change log and svn revision numbers in the JIRA
> comments so that someone investigating the JIRA or the svn revision will be
> able to figureout what is happening either from the JIRA or the svn change
> log without much difficulty.  I thought this is complete & sufficient and is
> definitely an improvement on what was followed earlier.  For those revisions
> that are not related to a JIRA, I have included a description of the change
> in the change log itself.  Since no one had any complaints on this, I have
> followed it till now.  Adding full details (or a summary) of the change to
> the change log would not be much of a overhead and it amounts to one more
> Copy+Paste.  I agree that adding a summary of changes along with a link to
> where full details can be found (like the JIRA number) is definitely a
> further improvement and will be followed from now on.  After all, all our
> efforts are to improve things and make life easy (for a lot of others).
>
> --vamsi
>
> On 11/28/06, Jason Dillon <jason@planet57.com> wrote:
> >
> > Please... please... please... put more context into the commit log.
> >
> > "MapEditor does not work" tells me nothing about what was actually
> > changed.  When auditing changes it helps to have some context to the
> > change in the commit log so that its easy to comprehend what the
> > change was, with out having to dig around, or bounce into JIRA, etc.
> >
> > I've mentioned this a few times before... while its is good to
> > include the JIRA issue ID, only including that ID, or in this case
> > the ID and the issue subject, in the SVN commit message is not
> > sufficient.
> >
> > Please... please... please... try to put some more meaningful context
> > into commit messages.
> >
> > Thanks,
> >
> > --jason
> >
> >
> > On Nov 27, 2006, at 2:54 AM, vamsic007@apache.org wrote:
> >
> > > Author: vamsic007
> > > Date: Mon Nov 27 02:54:38 2006
> > > New Revision: 479586
> > >
> > > URL: http://svn.apache.org/viewvc?view=rev&rev=479586
> > > Log:
> > > GERONIMO-2458 MapEditor does not work
> > >
> > > Modified:
> > >     geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> > > apache/geronimo/common/propertyeditor/MapEditor.java
> > >
> > > Modified: geronimo/server/trunk/modules/geronimo-common/src/main/
> > > java/org/apache/geronimo/common/propertyeditor/MapEditor.java
> > > URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/
> > > geronimo-common/src/main/java/org/apache/geronimo/common/
> > > propertyeditor/MapEditor.java?view=diff&rev=479586&r1=479585&r2=479586
> >
> > > ======================================================================
> > > ========
> > > --- geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> > > apache/geronimo/common/propertyeditor/MapEditor.java (original)
> > > +++ geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> > > apache/geronimo/common/propertyeditor/MapEditor.java Mon Nov 27
> > > 02:54:38 2006
> > > @@ -19,17 +19,22 @@
> > >
> > >  import java.io.ByteArrayInputStream;
> > >  import java.io.IOException;
> > > +import java.util.Iterator;
> > >  import java.util.Properties;
> > >  import java.util.Map;
> > >
> > > +import org.apache.commons.logging.Log ;
> > > +import org.apache.commons.logging.LogFactory;
> > > +
> > >  /**
> > > - * A property editor for {@link java.util.Properties}.
> > > + * A property editor for {@link java.util.Map}.
> > >   *
> > >   * @version $Rev$ $Date$
> > >   */
> > >  public class MapEditor
> > >     extends TextPropertyEditorSupport
> > >  {
> > > +    private static final Log log = LogFactory.getLog
> > > (MapEditor.class);
> > >      /**
> > >       *
> > >       * @throws PropertyEditorException  An IOException occured.
> > > @@ -50,11 +55,30 @@
> > >          Map map = (Map) getValue();
> > >          if (!(map instanceof Properties)) {
> > >              Properties p = new Properties();
> > > -            if (map != null) {
> > > -                p.putAll(map);
> > > +            if(map != null) {
> > > +                if(!map.containsKey(null) && !map.containsValue
> > > (null)) {
> > > +                    p.putAll(map);
> > > +                } else {
> > > +                    // Map contains null keys or values.  Replace
> > > null with empty string.
> > > +                    log.warn("Map contains null keys or values.
> > > Replacing null values with empty string.");
> > > +                    for(Iterator itr = map.entrySet().iterator();
> > > itr.hasNext(); ) {
> > > +                        Map.Entry entry = (Map.Entry) itr.next();
> > > +                        Object key = entry.getKey();
> > > +                        Object value = entry.getValue();
> > > +                        if(key == null) {
> > > +                            key = "";
> > > +                        }
> > > +                        if(value == null) {
> > > +                            value = "";
> > > +                        }
> > > +                        p.put(key, value);
> > > +                    }
> > > +                }
> > > +                map = p;
> > >              }
> > > -            map = p;
> > >          }
> > > -        return map.toString();
> > > +        PropertiesEditor pe = new PropertiesEditor();
> > > +        pe.setValue(map);
> > > +        return pe.getAsText();
> > >      }
> > >  }
> > >
> > >
> >
> >
>
>

Mime
View raw message