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 <[EMAIL PROTECTED]> 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 < [EMAIL PROTECTED]> 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, [EMAIL PROTECTED] 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 [EMAIL PROTECTED] java.util.Properties}.
> + * A property editor for [EMAIL PROTECTED] 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();
> }
> }
>
>
