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 05:57:24 GMT
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