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:25:52 GMT
Jason,

Don't worry.  I did not take it as personal.  Infact I am glad that you
commented on my practice and it will help in improving things.  This
discussion would be fruitful if we suggest some best practices for
Geronimo.  As I have already mentioned, "all our efforts are to improve
things and make life easy (for a lot of others)". :o)

--vamsi

On 11/28/06, Jason Dillon <jason@planet57.com> wrote:
>
> Aight... well, I still think that brief change context should be included
> in svn commits.  I have said it before... and no doubt I will say it again
> sometime.
> Anyways, nothing personal, not picking on you... was just providing some
> comments to help improve the ability to provide oversight of svn via change
> log emails.
>
> --jason
>
>
> On Nov 27, 2006, at 10:08 PM, Vamsavardhana Reddy wrote:
>
> 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