Moving this back to the mailing list...

On Jan 15, 2007, at 5:34 PM, Adrian Crum wrote:

David E. Jones wrote:
When reviewing a patch lines with formatting changes only are EXTREMELY time consuming, and the patch attached that issue is a good example of a painful one. For each of those line you have to stare at it looking at every character trying to figure out what the point of the bloody change was, or to make sure that the change is "invisible"...

That really puzzles me. You keep mentioning formatting changes when I don't see any. I didn't make any formatting changes when modifying the files. Even now when I look at the patch in Jira, it shows only the lines I changed in the file.

*shrug*

As others may have the same question I'll answer it on the mailing list. Below is the first section from the framework_v2.patch file on the [https://issues.apache.org/jira/browse/OFBIZ-605] issue.

Each change is marked with a "-" for a line removed or a "+" for a line added.

First Set: formatting, or "invisible" change

Second Set: add comment (not necessary, BTW as it describes something that no longer exists; if people want to see old stuff they can look at the SVN history); remove tabstyles.css link tag

Third Set: formatting, or "invisible" change

Of all of these changes, only the "remove tabstyles.css link tag" was actually intended and necessary, but getting to this fact when reviewing the changes takes some time... this making it more difficult to review, commit, etc.

People tend to slip things into patches accidentally all the time. I'm tempted to begun the voting process for a new policy that says that if there is anything in the patch file not described in the summary of the patch, then it will be rejected...

-David


========================
Index: common/webcommon/includes/header.ftl
===================================================================
--- common/webcommon/includes/header.ftl        (revision 494159)
+++ common/webcommon/includes/header.ftl        (working copy)
@@ -27,7 +27,7 @@
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8"/> <title>${layoutSettings.companyName}: <#if (page.titleProperty)? has_content>${uiLabelMap[page.titleProperty]}<#else>${(page.title)? if_exists}</#if></title> - <link rel="shortcut icon" href="<@ofbizContentUrl>/images/ ofbiz.ico</@ofbizContentUrl>" /> + <link rel="shortcut icon" href="<@ofbizContentUrl>/images/ ofbiz.ico</@ofbizContentUrl>" /> <script language="javascript" src="<@ofbizContentUrl>/images/ calendar1.js</@ofbizContentUrl>" type="text/javascript"></script> <script language="javascript" src="<@ofbizContentUrl>/images/ selectall.js</@ofbizContentUrl>" type="text/javascript"></script> <script language="javascript" src="<@ofbizContentUrl>/images/ fieldlookup.js</@ofbizContentUrl>" type="text/javascript"></script>
@@ -37,8 +37,8 @@
<link rel="stylesheet" href="<@ofbizContentUrl>$ {styleSheet}</@ofbizContentUrl>" type="text/css"/>
         </#list>
     <#else>
+        <#-- tabstyles.css has been moved to maincss.css -->
<link rel="stylesheet" href="<@ofbizContentUrl>/images/ maincss.css</@ofbizContentUrl>" type="text/css"/> - <link rel="stylesheet" href="<@ofbizContentUrl>/images/ tabstyles.css</@ofbizContentUrl>" type="text/css"/>
     </#if>
     ${layoutSettings.extraHead?if_exists}
</head>
@@ -51,7 +51,7 @@
         <tr>
           <#if layoutSettings.headerImageUrl?exists>
<td align="left" width="1%"><img alt="$ {layoutSettings.companyName}" src="<@ofbizContentUrl>$ {layoutSettings.headerImageUrl}</@ofbizContentUrl>"/></td>
-          </#if>
+          </#if>
<td align="right" width="1%" nowrap="nowrap" <#if layoutSettings.headerRightBackgroundUrl?has_content>background="$ {layoutSettings.headerRightBackgroundUrl}"</#if>>
             <div class="insideHeaderText">
             <#if person?has_content>
======================

Attachment: smime.p7s
Description: S/MIME cryptographic signature

Reply via email to