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();
> > }
> > }
> >
> >
>
>
|