geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Donald Woods <drw_...@yahoo.com>
Subject Re: svn commit: r479586 - /geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Date Mon, 27 Nov 2006 20:56:41 GMT
Providing a link to the JIRA makes more sense, as it can contain the 
full description of the fix, which releases it went in, any 
coreq/prereqs like OpenEJB or TranQL changes, if multiple patches and/or 
commits were required, .....

As long as everyone adds real problem and solution descriptions in the 
JIRAs, I'm fine with only providing a link back to it in a commit, which 
lots of people before Vamsi have done in the past without complaint :-)


-Donald


Jason Dillon wrote:
> As I mentioned... I really do not want to have to bounce from commit 
> messages to JIRA to provide oversight into the changes going into our 
> SVN repo.
> 
> Also, when something does break, I am going to want to look back through 
> the SVN logs and find meaningful context to changes.  In this case 
> "GERONIMO-2458 MapEditor does not work" does not tell me what was 
> changed, only tells me what was wrong.
> 
> IMO SVN is the definitive location for changes... and those changes 
> should be properly adorned with context of the change.
> 
> --jason
> 
> 
> On Nov 27, 2006, at 12:08 PM, Donald Woods wrote:
> 
>> The problem description and fix details were included in the JIRA by 
>> Vamsi as they should be - 
>> http://issues.apache.org/jira/browse/GERONIMO-2458
>>
>>
>> -Donald
>>
>>
>> Jason Dillon 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