You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by "David E. Jones" <jo...@hotwaxmedia.com> on 2007/01/16 01:44:11 UTC

Re: OFBiz UI work

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>
======================


Re: OFBiz UI work

Posted by David Welton <da...@gmail.com>.
> So, yeah, that slows things down and is inconvenient, but keep in
> mind that a large portion of patches that come into OFBiz break
> existing functionality. This seems to happen around once a week or
> so, at least.

Automated testing should help with this.

If I may make a suggestion, it would be that instead of having a
policy, that you trust the good sense of the OFBiz team.  If a patch
has a bunch of changes that don't relate to the code, then a developer
ought to be able to say "please don't do that, here's why" (in a FAQ?)
or not, depending on the situation at hand.

-- 
David N. Welton
 - http://www.dedasys.com/davidw/

Linux, Open Source Consulting
 - http://www.dedasys.com/

Re: OFBiz UI work

Posted by Guido Amarilla <ga...@gmail.com>.
OK

2007/1/16, David E. Jones <jo...@hotwaxmedia.com>:
>
> On Jan 16, 2007, at 5:11 AM, Guido Amarilla wrote:
>
> > BTW, I did my homework and faxed a signed iCLA to The Apache
> > Foundation.
>
> Not that it's a problem in any way, but why was it that you sent in
> an iCLA?
>
> Unless you are becoming a committer at the ASF or had contributed pre-
> ASF code to OFBiz (which has already been thoroughly reviewed and
> taken care of, as of a few months ago).
>
> -David
>
>
>
>


-- 
Guido Amarilla

Re: OFBiz UI work

Posted by "David E. Jones" <jo...@hotwaxmedia.com>.
On Jan 16, 2007, at 5:11 AM, Guido Amarilla wrote:

> BTW, I did my homework and faxed a signed iCLA to The Apache  
> Foundation.

Not that it's a problem in any way, but why was it that you sent in  
an iCLA?

Unless you are becoming a committer at the ASF or had contributed pre- 
ASF code to OFBiz (which has already been thoroughly reviewed and  
taken care of, as of a few months ago).

-David


Re: OFBiz UI work

Posted by Jacques Le Roux <ja...@les7arts.com>.
Guido,

From: "Guido Amarilla" <ga...@gmail.com>
> Jacques,
> That´s what I was talking about. You just should have rejected my
> patch explaining the reason and I should have corrected it and
> re-submitting.

Yes, seems the better way indeed. Actually I did not notice that this was new files. It's only after Marco Risaliti reminder on
another commit that I noticed the problem and corrected it.

> BTW, I did my homework and faxed a signed iCLA to The Apache Foundation.

Great !

Jaques

> Guido.
>
> 2007/1/16, Jacques Le Roux <ja...@les7arts.com>:
> > Guido,
> >
> > BTW, next time you propose a patch with new files in it, please don't forget to add the ASL 2 headers in them, thanks.
> >
> > This of course is true for everybody ;o)
> >
> > Jacques
> >
> > ----- Original Message -----
> > From: "Guido Amarilla" <ga...@gmail.com>
> > To: <de...@ofbiz.apache.org>
> > Sent: Tuesday, January 16, 2007 5:58 AM
> > Subject: Re: OFBiz UI work
> >
> >
> > > I´m giving my opinion, because it seems obvious to me that if you
> > > reject a patch, the contributor will correct it in the 99% of the
> > > times...and will learn, so the next time he probably won´t commit the
> > > same mistakes.
> > > This way we reduce the effort to the few commiters increasing the
> > > chances for them to commit other patches....
> > >
> > > Stronger policies: +1
> > >
> > > 2007/1/16, David E. Jones <jo...@hotwaxmedia.com>:
> > > >
> > > > Ummm... what do you think when you think of a rejection policy? We
> > > > already have patch rejection policies when problems are found...
> > > >
> > > > -David
> > > >
> > > >
> > > > On Jan 15, 2007, at 9:44 PM, Chris Howe wrote:
> > > >
> > > > > I agree.  I think it's better that we strongly
> > > > > encourage certain practices (as we are doing now) and
> > > > > bring people to notice when those practices could be
> > > > > improved, but rejection policies put your _means
> > > > > perspective ahead of your _ends.  Contributions may be
> > > > > easier to review, but I can gaurantee you with
> > > > > rejection policies, you will then have less of it to
> > > > > review and thereby less solutions making it back into
> > > > > the project.
> > > > >
> > > > > --- "David E. Jones" <jo...@hotwaxmedia.com> wrote:
> > > > >
> > > > >>
> > > > >> Yes, we want more people, but I don't think anyone
> > > > >> wants
> > > > >> indiscriminate changes going into SVN! I hate
> > > > >> surprises when I check
> > > > >> out as much as the next guy, probably more than the
> > > > >> next guy in many
> > > > >> cases.
> > > > >>
> > > > >> Thinking about the next guy is the real key here.
> > > > >> You may want to get
> > > > >> your patches in ASAP, but if your patch breaks
> > > > >> existing code (for
> > > > >> example), then that needs to be fixed before the
> > > > >> commit is done
> > > > >> (either by the committer, the contributor, or an
> > > > >> interested third
> > > > >> party).
> > > > >>
> > > > >> So, yeah, that slows things down and is
> > > > >> inconvenient, but keep in
> > > > >> mind that a large portion of patches that come into
> > > > >> OFBiz break
> > > > >> existing functionality. This seems to happen around
> > > > >> once a week or
> > > > >> so, at least.
> > > > >>
> > > > >> It's great that people want to contribute, but in or
> > > > >> to contribute to
> > > > >> something as large and complex as OFBiz a large
> > > > >> amount of effort is
> > > > >> necessary, and anyone that wants to help out will
> > > > >> err on the side of
> > > > >> extra effort, caution and review, and consideration
> > > > >> of more general
> > > > >> requirements and designs rather just one's own
> > > > >> requirements.
> > > > >>
> > > > >> -David
> > > > >>
> > > > >>
> > > > >>
> > > > >> On Jan 15, 2007, at 7:42 PM, Jonathon -- Improov
> > > > >> wrote:
> > > > >>
> > > > >>> Heh. True. I definitely want MORE people involved
> > > > >> in OFBiz.
> > > > >>>
> > > > >>> But since I'm not a committer, I'd rather make
> > > > >> things easier for
> > > > >>> the committers. I have no control over how easy or
> > > > >> difficult it is
> > > > >>> to handle patches with "extra unintended
> > > > >> footprints".
> > > > >>>
> > > > >>> If I were a committer, I would try to
> > > > >> automatically filter out
> > > > >>> formatting changes in my audit, and then INCLUDE
> > > > >> the formatting
> > > > >>> changes. After all, there's no harm removing some
> > > > >> extra spaces at
> > > > >>> the end of lines, part of clean up.
> > > > >>>
> > > > >>> We often try to make things easier for the person
> > > > >> who is doing a
> > > > >>> task that we have no control over. Eg, I'd keep my
> > > > >> mouth really
> > > > >>> wide open for my dentist just so his vision is
> > > > >> 20/20, no arguments
> > > > >>> from me; I could afford to be slack about this
> > > > >> "mouth wide-open
> > > > >>> policy" if I were able to do the dentistry myself.
> > > > >>>
> > > > >>> But you're certainly right that it'll exclude some
> > > > >> people,
> > > > >>> especially folks who use editors that do not give
> > > > >> very much control
> > > > >>> to users.
> > > > >>>
> > > > >>> Jonathon
> > > > >>>
> > > > >>> Chris Howe wrote:
> > > > >>>> I don't know that rejection policies are very
> > > > >>>> community friendly.  Treating SVN code changes
> > > > >> like
> > > > >>>> the keeper of the Bridge of Death (Monty Python
> > > > >>>> Reference, smile) may find less people willing to
> > > > >> do
> > > > >>>> in this do-ocracy.  Many of us rather like what's
> > > > >> left
> > > > >>>> of our anarco-sydicalist commune (oh look,
> > > > >> there's
> > > > >>>> another :-) ).
> > > > >>>> --- Jonathon -- Improov <jo...@improov.com> wrote:
> > > > >>>>> David,
> > > > >>>>>
> > > > >>>>> I agree with the rejection policy.
> > > > >>>>>
> > > > >>>>> One suggestion on procedure to take when
> > > > >>>>> self-reviewing a patch before submitting it.
> > > > >> Look at
> > > > >>>>> the diff to see if there are changes we cannot
> > > > >> account
> > > > >>>>> for. Using KDiff also makes things easier, even
> > > > >> when dealing with
> > > > >>>>> formatting changes.
> > > > >>>>>
> > > > >>>>> In Emacs, it's also easy to go through every set
> > > > >> of
> > > > >>>>> changes, do an interactive-search for 1st entry
> > > > >> of each set, and
> > > > >>>>> see if the 2nd entry is
> > > > >>>>> highlighted similarly. Meaning, if it is
> > > > >> highlighted similarly,
> > > > >>>>> the set of changes is bogus
> > > > >>>>> (formatting only).
> > > > >>>>>
> > > > >>>>> In general, we should submit patches that are
> > > > >>>>> intentional, ie just the intended changes only.
> > > > >> We should not
> > > > >>>>> submit patches that contain unintended
> > > > >>>>> changes that have extra unintended footprints.
> > > > >>>>>
> > > > >>>>> Jonathon
> > > > >>>>>
> > > > >>>>> David E. Jones wrote:
> > > > >>>>>> 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
> > > > >>
> > > > > === message truncated ===
> > > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > > --
> > > Guido Amarilla
> >
> >
>
>
> -- 
> Guido Amarilla


Re: OFBiz UI work

Posted by Guido Amarilla <ga...@gmail.com>.
Jacques,
That´s what I was talking about. You just should have rejected my
patch explaining the reason and I should have corrected it and
re-submitting.

BTW, I did my homework and faxed a signed iCLA to The Apache Foundation.

Guido.

2007/1/16, Jacques Le Roux <ja...@les7arts.com>:
> Guido,
>
> BTW, next time you propose a patch with new files in it, please don't forget to add the ASL 2 headers in them, thanks.
>
> This of course is true for everybody ;o)
>
> Jacques
>
> ----- Original Message -----
> From: "Guido Amarilla" <ga...@gmail.com>
> To: <de...@ofbiz.apache.org>
> Sent: Tuesday, January 16, 2007 5:58 AM
> Subject: Re: OFBiz UI work
>
>
> > I´m giving my opinion, because it seems obvious to me that if you
> > reject a patch, the contributor will correct it in the 99% of the
> > times...and will learn, so the next time he probably won´t commit the
> > same mistakes.
> > This way we reduce the effort to the few commiters increasing the
> > chances for them to commit other patches....
> >
> > Stronger policies: +1
> >
> > 2007/1/16, David E. Jones <jo...@hotwaxmedia.com>:
> > >
> > > Ummm... what do you think when you think of a rejection policy? We
> > > already have patch rejection policies when problems are found...
> > >
> > > -David
> > >
> > >
> > > On Jan 15, 2007, at 9:44 PM, Chris Howe wrote:
> > >
> > > > I agree.  I think it's better that we strongly
> > > > encourage certain practices (as we are doing now) and
> > > > bring people to notice when those practices could be
> > > > improved, but rejection policies put your _means
> > > > perspective ahead of your _ends.  Contributions may be
> > > > easier to review, but I can gaurantee you with
> > > > rejection policies, you will then have less of it to
> > > > review and thereby less solutions making it back into
> > > > the project.
> > > >
> > > > --- "David E. Jones" <jo...@hotwaxmedia.com> wrote:
> > > >
> > > >>
> > > >> Yes, we want more people, but I don't think anyone
> > > >> wants
> > > >> indiscriminate changes going into SVN! I hate
> > > >> surprises when I check
> > > >> out as much as the next guy, probably more than the
> > > >> next guy in many
> > > >> cases.
> > > >>
> > > >> Thinking about the next guy is the real key here.
> > > >> You may want to get
> > > >> your patches in ASAP, but if your patch breaks
> > > >> existing code (for
> > > >> example), then that needs to be fixed before the
> > > >> commit is done
> > > >> (either by the committer, the contributor, or an
> > > >> interested third
> > > >> party).
> > > >>
> > > >> So, yeah, that slows things down and is
> > > >> inconvenient, but keep in
> > > >> mind that a large portion of patches that come into
> > > >> OFBiz break
> > > >> existing functionality. This seems to happen around
> > > >> once a week or
> > > >> so, at least.
> > > >>
> > > >> It's great that people want to contribute, but in or
> > > >> to contribute to
> > > >> something as large and complex as OFBiz a large
> > > >> amount of effort is
> > > >> necessary, and anyone that wants to help out will
> > > >> err on the side of
> > > >> extra effort, caution and review, and consideration
> > > >> of more general
> > > >> requirements and designs rather just one's own
> > > >> requirements.
> > > >>
> > > >> -David
> > > >>
> > > >>
> > > >>
> > > >> On Jan 15, 2007, at 7:42 PM, Jonathon -- Improov
> > > >> wrote:
> > > >>
> > > >>> Heh. True. I definitely want MORE people involved
> > > >> in OFBiz.
> > > >>>
> > > >>> But since I'm not a committer, I'd rather make
> > > >> things easier for
> > > >>> the committers. I have no control over how easy or
> > > >> difficult it is
> > > >>> to handle patches with "extra unintended
> > > >> footprints".
> > > >>>
> > > >>> If I were a committer, I would try to
> > > >> automatically filter out
> > > >>> formatting changes in my audit, and then INCLUDE
> > > >> the formatting
> > > >>> changes. After all, there's no harm removing some
> > > >> extra spaces at
> > > >>> the end of lines, part of clean up.
> > > >>>
> > > >>> We often try to make things easier for the person
> > > >> who is doing a
> > > >>> task that we have no control over. Eg, I'd keep my
> > > >> mouth really
> > > >>> wide open for my dentist just so his vision is
> > > >> 20/20, no arguments
> > > >>> from me; I could afford to be slack about this
> > > >> "mouth wide-open
> > > >>> policy" if I were able to do the dentistry myself.
> > > >>>
> > > >>> But you're certainly right that it'll exclude some
> > > >> people,
> > > >>> especially folks who use editors that do not give
> > > >> very much control
> > > >>> to users.
> > > >>>
> > > >>> Jonathon
> > > >>>
> > > >>> Chris Howe wrote:
> > > >>>> I don't know that rejection policies are very
> > > >>>> community friendly.  Treating SVN code changes
> > > >> like
> > > >>>> the keeper of the Bridge of Death (Monty Python
> > > >>>> Reference, smile) may find less people willing to
> > > >> do
> > > >>>> in this do-ocracy.  Many of us rather like what's
> > > >> left
> > > >>>> of our anarco-sydicalist commune (oh look,
> > > >> there's
> > > >>>> another :-) ).
> > > >>>> --- Jonathon -- Improov <jo...@improov.com> wrote:
> > > >>>>> David,
> > > >>>>>
> > > >>>>> I agree with the rejection policy.
> > > >>>>>
> > > >>>>> One suggestion on procedure to take when
> > > >>>>> self-reviewing a patch before submitting it.
> > > >> Look at
> > > >>>>> the diff to see if there are changes we cannot
> > > >> account
> > > >>>>> for. Using KDiff also makes things easier, even
> > > >> when dealing with
> > > >>>>> formatting changes.
> > > >>>>>
> > > >>>>> In Emacs, it's also easy to go through every set
> > > >> of
> > > >>>>> changes, do an interactive-search for 1st entry
> > > >> of each set, and
> > > >>>>> see if the 2nd entry is
> > > >>>>> highlighted similarly. Meaning, if it is
> > > >> highlighted similarly,
> > > >>>>> the set of changes is bogus
> > > >>>>> (formatting only).
> > > >>>>>
> > > >>>>> In general, we should submit patches that are
> > > >>>>> intentional, ie just the intended changes only.
> > > >> We should not
> > > >>>>> submit patches that contain unintended
> > > >>>>> changes that have extra unintended footprints.
> > > >>>>>
> > > >>>>> Jonathon
> > > >>>>>
> > > >>>>> David E. Jones wrote:
> > > >>>>>> 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
> > > >>
> > > > === message truncated ===
> > > >
> > >
> > >
> > >
> > >
> >
> >
> > --
> > Guido Amarilla
>
>


-- 
Guido Amarilla

Re: OFBiz UI work

Posted by Jacques Le Roux <ja...@les7arts.com>.
Guido,

BTW, next time you propose a patch with new files in it, please don't forget to add the ASL 2 headers in them, thanks.

This of course is true for everybody ;o)

Jacques

----- Original Message ----- 
From: "Guido Amarilla" <ga...@gmail.com>
To: <de...@ofbiz.apache.org>
Sent: Tuesday, January 16, 2007 5:58 AM
Subject: Re: OFBiz UI work


> I´m giving my opinion, because it seems obvious to me that if you
> reject a patch, the contributor will correct it in the 99% of the
> times...and will learn, so the next time he probably won´t commit the
> same mistakes.
> This way we reduce the effort to the few commiters increasing the
> chances for them to commit other patches....
>
> Stronger policies: +1
>
> 2007/1/16, David E. Jones <jo...@hotwaxmedia.com>:
> >
> > Ummm... what do you think when you think of a rejection policy? We
> > already have patch rejection policies when problems are found...
> >
> > -David
> >
> >
> > On Jan 15, 2007, at 9:44 PM, Chris Howe wrote:
> >
> > > I agree.  I think it's better that we strongly
> > > encourage certain practices (as we are doing now) and
> > > bring people to notice when those practices could be
> > > improved, but rejection policies put your _means
> > > perspective ahead of your _ends.  Contributions may be
> > > easier to review, but I can gaurantee you with
> > > rejection policies, you will then have less of it to
> > > review and thereby less solutions making it back into
> > > the project.
> > >
> > > --- "David E. Jones" <jo...@hotwaxmedia.com> wrote:
> > >
> > >>
> > >> Yes, we want more people, but I don't think anyone
> > >> wants
> > >> indiscriminate changes going into SVN! I hate
> > >> surprises when I check
> > >> out as much as the next guy, probably more than the
> > >> next guy in many
> > >> cases.
> > >>
> > >> Thinking about the next guy is the real key here.
> > >> You may want to get
> > >> your patches in ASAP, but if your patch breaks
> > >> existing code (for
> > >> example), then that needs to be fixed before the
> > >> commit is done
> > >> (either by the committer, the contributor, or an
> > >> interested third
> > >> party).
> > >>
> > >> So, yeah, that slows things down and is
> > >> inconvenient, but keep in
> > >> mind that a large portion of patches that come into
> > >> OFBiz break
> > >> existing functionality. This seems to happen around
> > >> once a week or
> > >> so, at least.
> > >>
> > >> It's great that people want to contribute, but in or
> > >> to contribute to
> > >> something as large and complex as OFBiz a large
> > >> amount of effort is
> > >> necessary, and anyone that wants to help out will
> > >> err on the side of
> > >> extra effort, caution and review, and consideration
> > >> of more general
> > >> requirements and designs rather just one's own
> > >> requirements.
> > >>
> > >> -David
> > >>
> > >>
> > >>
> > >> On Jan 15, 2007, at 7:42 PM, Jonathon -- Improov
> > >> wrote:
> > >>
> > >>> Heh. True. I definitely want MORE people involved
> > >> in OFBiz.
> > >>>
> > >>> But since I'm not a committer, I'd rather make
> > >> things easier for
> > >>> the committers. I have no control over how easy or
> > >> difficult it is
> > >>> to handle patches with "extra unintended
> > >> footprints".
> > >>>
> > >>> If I were a committer, I would try to
> > >> automatically filter out
> > >>> formatting changes in my audit, and then INCLUDE
> > >> the formatting
> > >>> changes. After all, there's no harm removing some
> > >> extra spaces at
> > >>> the end of lines, part of clean up.
> > >>>
> > >>> We often try to make things easier for the person
> > >> who is doing a
> > >>> task that we have no control over. Eg, I'd keep my
> > >> mouth really
> > >>> wide open for my dentist just so his vision is
> > >> 20/20, no arguments
> > >>> from me; I could afford to be slack about this
> > >> "mouth wide-open
> > >>> policy" if I were able to do the dentistry myself.
> > >>>
> > >>> But you're certainly right that it'll exclude some
> > >> people,
> > >>> especially folks who use editors that do not give
> > >> very much control
> > >>> to users.
> > >>>
> > >>> Jonathon
> > >>>
> > >>> Chris Howe wrote:
> > >>>> I don't know that rejection policies are very
> > >>>> community friendly.  Treating SVN code changes
> > >> like
> > >>>> the keeper of the Bridge of Death (Monty Python
> > >>>> Reference, smile) may find less people willing to
> > >> do
> > >>>> in this do-ocracy.  Many of us rather like what's
> > >> left
> > >>>> of our anarco-sydicalist commune (oh look,
> > >> there's
> > >>>> another :-) ).
> > >>>> --- Jonathon -- Improov <jo...@improov.com> wrote:
> > >>>>> David,
> > >>>>>
> > >>>>> I agree with the rejection policy.
> > >>>>>
> > >>>>> One suggestion on procedure to take when
> > >>>>> self-reviewing a patch before submitting it.
> > >> Look at
> > >>>>> the diff to see if there are changes we cannot
> > >> account
> > >>>>> for. Using KDiff also makes things easier, even
> > >> when dealing with
> > >>>>> formatting changes.
> > >>>>>
> > >>>>> In Emacs, it's also easy to go through every set
> > >> of
> > >>>>> changes, do an interactive-search for 1st entry
> > >> of each set, and
> > >>>>> see if the 2nd entry is
> > >>>>> highlighted similarly. Meaning, if it is
> > >> highlighted similarly,
> > >>>>> the set of changes is bogus
> > >>>>> (formatting only).
> > >>>>>
> > >>>>> In general, we should submit patches that are
> > >>>>> intentional, ie just the intended changes only.
> > >> We should not
> > >>>>> submit patches that contain unintended
> > >>>>> changes that have extra unintended footprints.
> > >>>>>
> > >>>>> Jonathon
> > >>>>>
> > >>>>> David E. Jones wrote:
> > >>>>>> 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
> > >>
> > > === message truncated ===
> > >
> >
> >
> >
> >
>
>
> -- 
> Guido Amarilla


Re: OFBiz UI work

Posted by Guido Amarilla <ga...@gmail.com>.
I´m giving my opinion, because it seems obvious to me that if you
reject a patch, the contributor will correct it in the 99% of the
times...and will learn, so the next time he probably won´t commit the
same mistakes.
This way we reduce the effort to the few commiters increasing the
chances for them to commit other patches....

Stronger policies: +1

2007/1/16, David E. Jones <jo...@hotwaxmedia.com>:
>
> Ummm... what do you think when you think of a rejection policy? We
> already have patch rejection policies when problems are found...
>
> -David
>
>
> On Jan 15, 2007, at 9:44 PM, Chris Howe wrote:
>
> > I agree.  I think it's better that we strongly
> > encourage certain practices (as we are doing now) and
> > bring people to notice when those practices could be
> > improved, but rejection policies put your _means
> > perspective ahead of your _ends.  Contributions may be
> > easier to review, but I can gaurantee you with
> > rejection policies, you will then have less of it to
> > review and thereby less solutions making it back into
> > the project.
> >
> > --- "David E. Jones" <jo...@hotwaxmedia.com> wrote:
> >
> >>
> >> Yes, we want more people, but I don't think anyone
> >> wants
> >> indiscriminate changes going into SVN! I hate
> >> surprises when I check
> >> out as much as the next guy, probably more than the
> >> next guy in many
> >> cases.
> >>
> >> Thinking about the next guy is the real key here.
> >> You may want to get
> >> your patches in ASAP, but if your patch breaks
> >> existing code (for
> >> example), then that needs to be fixed before the
> >> commit is done
> >> (either by the committer, the contributor, or an
> >> interested third
> >> party).
> >>
> >> So, yeah, that slows things down and is
> >> inconvenient, but keep in
> >> mind that a large portion of patches that come into
> >> OFBiz break
> >> existing functionality. This seems to happen around
> >> once a week or
> >> so, at least.
> >>
> >> It's great that people want to contribute, but in or
> >> to contribute to
> >> something as large and complex as OFBiz a large
> >> amount of effort is
> >> necessary, and anyone that wants to help out will
> >> err on the side of
> >> extra effort, caution and review, and consideration
> >> of more general
> >> requirements and designs rather just one's own
> >> requirements.
> >>
> >> -David
> >>
> >>
> >>
> >> On Jan 15, 2007, at 7:42 PM, Jonathon -- Improov
> >> wrote:
> >>
> >>> Heh. True. I definitely want MORE people involved
> >> in OFBiz.
> >>>
> >>> But since I'm not a committer, I'd rather make
> >> things easier for
> >>> the committers. I have no control over how easy or
> >> difficult it is
> >>> to handle patches with "extra unintended
> >> footprints".
> >>>
> >>> If I were a committer, I would try to
> >> automatically filter out
> >>> formatting changes in my audit, and then INCLUDE
> >> the formatting
> >>> changes. After all, there's no harm removing some
> >> extra spaces at
> >>> the end of lines, part of clean up.
> >>>
> >>> We often try to make things easier for the person
> >> who is doing a
> >>> task that we have no control over. Eg, I'd keep my
> >> mouth really
> >>> wide open for my dentist just so his vision is
> >> 20/20, no arguments
> >>> from me; I could afford to be slack about this
> >> "mouth wide-open
> >>> policy" if I were able to do the dentistry myself.
> >>>
> >>> But you're certainly right that it'll exclude some
> >> people,
> >>> especially folks who use editors that do not give
> >> very much control
> >>> to users.
> >>>
> >>> Jonathon
> >>>
> >>> Chris Howe wrote:
> >>>> I don't know that rejection policies are very
> >>>> community friendly.  Treating SVN code changes
> >> like
> >>>> the keeper of the Bridge of Death (Monty Python
> >>>> Reference, smile) may find less people willing to
> >> do
> >>>> in this do-ocracy.  Many of us rather like what's
> >> left
> >>>> of our anarco-sydicalist commune (oh look,
> >> there's
> >>>> another :-) ).
> >>>> --- Jonathon -- Improov <jo...@improov.com> wrote:
> >>>>> David,
> >>>>>
> >>>>> I agree with the rejection policy.
> >>>>>
> >>>>> One suggestion on procedure to take when
> >>>>> self-reviewing a patch before submitting it.
> >> Look at
> >>>>> the diff to see if there are changes we cannot
> >> account
> >>>>> for. Using KDiff also makes things easier, even
> >> when dealing with
> >>>>> formatting changes.
> >>>>>
> >>>>> In Emacs, it's also easy to go through every set
> >> of
> >>>>> changes, do an interactive-search for 1st entry
> >> of each set, and
> >>>>> see if the 2nd entry is
> >>>>> highlighted similarly. Meaning, if it is
> >> highlighted similarly,
> >>>>> the set of changes is bogus
> >>>>> (formatting only).
> >>>>>
> >>>>> In general, we should submit patches that are
> >>>>> intentional, ie just the intended changes only.
> >> We should not
> >>>>> submit patches that contain unintended
> >>>>> changes that have extra unintended footprints.
> >>>>>
> >>>>> Jonathon
> >>>>>
> >>>>> David E. Jones wrote:
> >>>>>> 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
> >>
> > === message truncated ===
> >
>
>
>
>


-- 
Guido Amarilla

Re: OFBiz UI work

Posted by Jacques Le Roux <ja...@les7arts.com>.
From: "Jonathon -- Improov" <jo...@improov.com>
> Chris has a point. There's definitely an implicit "rejection policy" already in place.
[...snip...]
 
> I've submitted about 3-4 patches, 3 (75-90%) of those early patches have wrong "path to file" (I 
> created patch from same folder as patched file, not from root with folders like application, 
> framework, etc). Jacopo mentioned it to me twice or thrice. I got the message.
> 
> But of course, there can be those who are completely out of line all the time, no matter what you 
> tell them.
> 
> Still, it could definitely save Jacopo A TON of time if he were officially allowed to respond with 
> short default message: "Sorry, I can't look at your patch at all due to time constraints. Please 
> refer to best practices page to correct your patch first. Or give patch to some kind community 
> member who will correct it for you. Sorry for the inconvenience."

BTW it was not Jacopo but me ;o)

Jacques
 
> Now that would be better than: "Sorry, patch no good. Please try again." I don't see anyone ever 
> doing this, though.
> 
> Jonathon


Re: OFBiz UI work

Posted by Jonathon -- Improov <jo...@improov.com>.
Chris has a point. There's definitely an implicit "rejection policy" already in place.

Given a strong rejection policy, there are 2 opposite spectrums to its implementation. You can 
follow the Singapore method that says "Our fatherland says this, so this is it. You cannot make a 
U-turn because the law book didn't say you can do so on that road". Or you can say "please try to 
do this, but we'll look at your case and let you through the first few times". (Frankly, the 1st 
method is LAZY; the 2nd takes more effort and is human-friendly.)

I've submitted about 3-4 patches, 3 (75-90%) of those early patches have wrong "path to file" (I 
created patch from same folder as patched file, not from root with folders like application, 
framework, etc). Jacopo mentioned it to me twice or thrice. I got the message.

But of course, there can be those who are completely out of line all the time, no matter what you 
tell them.

Still, it could definitely save Jacopo A TON of time if he were officially allowed to respond with 
short default message: "Sorry, I can't look at your patch at all due to time constraints. Please 
refer to best practices page to correct your patch first. Or give patch to some kind community 
member who will correct it for you. Sorry for the inconvenience."

Now that would be better than: "Sorry, patch no good. Please try again." I don't see anyone ever 
doing this, though.

Jonathon

Chris Howe wrote:
> Being tempted to ask for a vote, insinuates that you
> want to condone a practice whereby if you go to review
> a patch like in Adrian's case and it has superflous
> formatting changes, that you would delete it from JIRA
> in a "sorry kid, you're wasting my time" dismissal
> (email impressions added for levity).  Anything less
> is what is currently going on and wouldn't need a
> vote.  People aren't tempted to vote on the status
> quo.

Re: OFBiz UI work, Plus New Revelations

Posted by Adrian Crum <ad...@hlmksw.com>.
Jonathon -- Improov wrote:
> Adrian,
> 
> I don't understand what you mean by "OFBiz servers are spewing out 
> unnecessary stuff". Those FTLs with unnecessary whitespaces are probably 
> coded that way (by mistake or otherwise), not generated and spewed by 
> some servers.

The point I was making is that any unnecessary white space in FTL files 
translates into unnecessary white space being served to web clients.

I was merely sharing an observation. I'm not suggesting that we change the best 
practices.


Re: OFBiz UI work, Plus New Revelations

Posted by Jonathon -- Improov <jo...@improov.com>.
Adrian,

I don't understand what you mean by "OFBiz servers are spewing out unnecessary stuff". Those FTLs 
with unnecessary whitespaces are probably coded that way (by mistake or otherwise), not generated 
and spewed by some servers.

As for indentation, I recall the "best practices" page for contributors stating that all tabs 
should be converted to spaces. That's quite the norm in many, if not all, projects. You mean you 
found tabs in FTL sources?

In any case, all these don't affect me. Use Emacs! I don't remove or change any whitespaces, not 
even change tabs to spaces.

Far as I'm concerned, I will continue to submit patches that have "intended changes only". I'm 
kinda obsessive compulsively clean and organized. I'd rather have say the original authors clean 
up the extra whitespaces, and check in with log saying "cleaned up whitespaces". My check in log 
will only be "fixed this bug", not "fixed this bug, plus cleaned up whitespaces".

But yeah, your editor isn't wrong to clean up those whitespaces. No wait. It's wrong. Or at least 
it's wrong not to allow you to configure that behavior.

Jonathon

Adrian Crum wrote:
> After spending some time examining the unintentional formatting changes 
> in my patch files, I discovered that my editor automatically strips off 
> unnecessary white space at the end of every line. I can't find a way to 
> shut it off, so I'll have to switch to another IDE.
> 
> At first I was upset that my editor would do such a thing without my 
> permission. Then I got to thinking that it makes a lot of sense. Less 
> unnecessary white space equals less fluff the compiler has to trudge 
> through and less fluff in HTML code.
> 
> Hey! Wait a second... many of those files that were unintentionally 
> formatted were FTL files. Does that mean that OFBiz servers are spewing 
> out unnecessary fluff? I viewed the page source on a typical OFBiz web 
> page and sure enough - OFBiz's markup has unnecessary white space at the 
> end of the lines.
> 
> Going through all of the FTL files and cleaning them up would be easy to 
> do with  a script or something, but the reduction in HTML output would 
> be small. Where I see a huge amount of unnecessary markup is with 
> indentation. Our four character indentation rule results in things like 
> a simple </div> tag being preceded by twelve to sixteen space 
> characters. Our servers are working very hard to output nicely indented 
> markup.
> 
> 


Re: OFBiz UI work, Plus New Revelations

Posted by Adrian Crum <ad...@hlmksw.com>.
Thanks Joe!

Joe Eckard wrote:

> 
> On Jan 19, 2007, at 4:43 PM, Adrian Crum wrote:
> 
>> I found on the Tomcat website a configuration setting for  compressing 
>> outgoing html/xml (http://tomcat.apache.org/tomcat-5.5- 
>> doc/config/http.html) but I don't where to configure it in the  
>> embedded version that OFBiz uses.
> 
> 
> 
> It's under http-connector (already set in the default config):
> 
> <property name="compressableMimeType" value="text/html,text/xml,text/ 
> plain"/>
> <property name="compression" value="on"/>
> 
> -Joe

Re: OFBiz UI work, Plus New Revelations

Posted by Joe Eckard <jo...@redrocketcorp.com>.
On Jan 19, 2007, at 4:43 PM, Adrian Crum wrote:

> I found on the Tomcat website a configuration setting for  
> compressing outgoing html/xml (http://tomcat.apache.org/tomcat-5.5- 
> doc/config/http.html) but I don't where to configure it in the  
> embedded version that OFBiz uses.


It's under http-connector (already set in the default config):

<property name="compressableMimeType" value="text/html,text/xml,text/ 
plain"/>
<property name="compression" value="on"/>

-Joe

Re: OFBiz UI work, Plus New Revelations

Posted by Adrian Crum <ad...@hlmksw.com>.
I found on the Tomcat website a configuration setting for compressing outgoing 
html/xml (http://tomcat.apache.org/tomcat-5.5-doc/config/http.html) but I don't 
where to configure it in the embedded version that OFBiz uses.


David E. Jones wrote:

> 
> The best way I've seen to handle this sort of thing is to take  
> advantage of the fact that pretty much all browsers support zipped  
> pages. I haven't set this sort of thing up in a LONG time, but there  
> are probably ways to do it with Tomcat, and definitely ways to do it  
> with the Apache web server (httpd).
> 
> -David
> 
> 
> On Jan 18, 2007, at 1:40 PM, Adrian Crum wrote:
> 
>> Just for grins, I inserted <#compress> </#compress> FTL directives  in 
>> the Party Manager FTL files to see how much smaller the markup  would 
>> be. Results:
>>
>> Before compress - 45k
>> After compress - 35k
>> 33% less markup.
>>
>> The drawback is, some of the layout seems to depend on some of the  
>> FTL whitespace, so the page's appearance changed a little.
>>
>>
>> Adrian Crum wrote:
>>
>>> After spending some time examining the unintentional formatting  
>>> changes in my patch files, I discovered that my editor  automatically 
>>> strips off unnecessary white space at the end of  every line. I can't 
>>> find a way to shut it off, so I'll have to  switch to another IDE.
>>> At first I was upset that my editor would do such a thing without  my 
>>> permission. Then I got to thinking that it makes a lot of  sense. 
>>> Less unnecessary white space equals less fluff the compiler  has to 
>>> trudge through and less fluff in HTML code.
>>> Hey! Wait a second... many of those files that were  unintentionally 
>>> formatted were FTL files. Does that mean that  OFBiz servers are 
>>> spewing out unnecessary fluff? I viewed the page  source on a typical 
>>> OFBiz web page and sure enough - OFBiz's  markup has unnecessary 
>>> white space at the end of the lines.
>>> Going through all of the FTL files and cleaning them up would be  
>>> easy to do with  a script or something, but the reduction in HTML  
>>> output would be small. Where I see a huge amount of unnecessary  
>>> markup is with indentation. Our four character indentation rule  
>>> results in things like a simple </div> tag being preceded by  twelve 
>>> to sixteen space characters. Our servers are working very  hard to 
>>> output nicely indented markup.
> 
> 

Re: OFBiz UI work, Plus New Revelations

Posted by Adrian Crum <ad...@hlmksw.com>.
Oh wait. I misread what you said. You're suggesting zipped pages sent out from 
the server that the browser can unzip. Gotcha. Great idea. Thanks.


Adrian Crum wrote:

> As I mentioned in another email, I was just making an observation. It's 
> food for thought.
> 
> The zipped pages browser setting doesn't address the fundamental issue I 
> presented: OFBiz servers are pushing out a lot of unnecessary markup.
> 
> It would be interesting to try out an OFBiz installation where 
> Freemarker/Tomcat/whatever is set up to compress ALL markup, then see 
> how much more responsive the web site is.
> 
> 
> David E. Jones wrote:
> 
>>
>> The best way I've seen to handle this sort of thing is to take  
>> advantage of the fact that pretty much all browsers support zipped  
>> pages. I haven't set this sort of thing up in a LONG time, but there  
>> are probably ways to do it with Tomcat, and definitely ways to do it  
>> with the Apache web server (httpd).
>>
>> -David
>>
>>
>> On Jan 18, 2007, at 1:40 PM, Adrian Crum wrote:
>>
>>> Just for grins, I inserted <#compress> </#compress> FTL directives  
>>> in the Party Manager FTL files to see how much smaller the markup  
>>> would be. Results:
>>>
>>> Before compress - 45k
>>> After compress - 35k
>>> 33% less markup.
>>>
>>> The drawback is, some of the layout seems to depend on some of the  
>>> FTL whitespace, so the page's appearance changed a little.
>>>
>>>
>>> Adrian Crum wrote:
>>>
>>>> After spending some time examining the unintentional formatting  
>>>> changes in my patch files, I discovered that my editor  
>>>> automatically strips off unnecessary white space at the end of  
>>>> every line. I can't find a way to shut it off, so I'll have to  
>>>> switch to another IDE.
>>>> At first I was upset that my editor would do such a thing without  
>>>> my permission. Then I got to thinking that it makes a lot of  sense. 
>>>> Less unnecessary white space equals less fluff the compiler  has to 
>>>> trudge through and less fluff in HTML code.
>>>> Hey! Wait a second... many of those files that were  unintentionally 
>>>> formatted were FTL files. Does that mean that  OFBiz servers are 
>>>> spewing out unnecessary fluff? I viewed the page  source on a 
>>>> typical OFBiz web page and sure enough - OFBiz's  markup has 
>>>> unnecessary white space at the end of the lines.
>>>> Going through all of the FTL files and cleaning them up would be  
>>>> easy to do with  a script or something, but the reduction in HTML  
>>>> output would be small. Where I see a huge amount of unnecessary  
>>>> markup is with indentation. Our four character indentation rule  
>>>> results in things like a simple </div> tag being preceded by  twelve 
>>>> to sixteen space characters. Our servers are working very  hard to 
>>>> output nicely indented markup.
>>
>>
>>
> 

Re: OFBiz UI work, Plus New Revelations

Posted by Jonathon -- Improov <jo...@improov.com>.
Adrian,

 > The zipped pages browser setting doesn't address the fundamental issue I
 > presented: OFBiz servers are pushing out a lot of unnecessary markup.

I'd guess there won't be too much unnecessary "whitespaces" (is this what you mean by unnecessary 
markup?). But I don't know. Someone could be crazy and code tons of unnecessary whitespaces into 
an FTL.

 > It would be interesting to try out an OFBiz installation where
 > Freemarker/Tomcat/whatever is set up to compress ALL markup, then see
 > how much more responsive the web site is.

Not by much if server-side processing is hefty. Savings from reduced time of transfer (of web 
content) is amortized in that case.

But in general, if you can, use compression. Or actually, for maximum compatibility with all 
browsers, maybe we shouldn't (we never know).

Jonathon

Adrian Crum wrote:
> As I mentioned in another email, I was just making an observation. It's 
> food for thought.
> 
> The zipped pages browser setting doesn't address the fundamental issue I 
> presented: OFBiz servers are pushing out a lot of unnecessary markup.
> 
> It would be interesting to try out an OFBiz installation where 
> Freemarker/Tomcat/whatever is set up to compress ALL markup, then see 
> how much more responsive the web site is.
> 
> 
> David E. Jones wrote:
>>
>> The best way I've seen to handle this sort of thing is to take  
>> advantage of the fact that pretty much all browsers support zipped  
>> pages. I haven't set this sort of thing up in a LONG time, but there  
>> are probably ways to do it with Tomcat, and definitely ways to do it  
>> with the Apache web server (httpd).
>>
>> -David
>>
>>
>> On Jan 18, 2007, at 1:40 PM, Adrian Crum wrote:
>>
>>> Just for grins, I inserted <#compress> </#compress> FTL directives  
>>> in the Party Manager FTL files to see how much smaller the markup  
>>> would be. Results:
>>>
>>> Before compress - 45k
>>> After compress - 35k
>>> 33% less markup.
>>>
>>> The drawback is, some of the layout seems to depend on some of the  
>>> FTL whitespace, so the page's appearance changed a little.
>>>
>>>
>>> Adrian Crum wrote:
>>>
>>>> After spending some time examining the unintentional formatting  
>>>> changes in my patch files, I discovered that my editor  
>>>> automatically strips off unnecessary white space at the end of  
>>>> every line. I can't find a way to shut it off, so I'll have to  
>>>> switch to another IDE.
>>>> At first I was upset that my editor would do such a thing without  
>>>> my permission. Then I got to thinking that it makes a lot of  sense. 
>>>> Less unnecessary white space equals less fluff the compiler  has to 
>>>> trudge through and less fluff in HTML code.
>>>> Hey! Wait a second... many of those files that were  unintentionally 
>>>> formatted were FTL files. Does that mean that  OFBiz servers are 
>>>> spewing out unnecessary fluff? I viewed the page  source on a 
>>>> typical OFBiz web page and sure enough - OFBiz's  markup has 
>>>> unnecessary white space at the end of the lines.
>>>> Going through all of the FTL files and cleaning them up would be  
>>>> easy to do with  a script or something, but the reduction in HTML  
>>>> output would be small. Where I see a huge amount of unnecessary  
>>>> markup is with indentation. Our four character indentation rule  
>>>> results in things like a simple </div> tag being preceded by  twelve 
>>>> to sixteen space characters. Our servers are working very  hard to 
>>>> output nicely indented markup.
>>
>>
> 
> 


Re: OFBiz UI work, Plus New Revelations

Posted by Adrian Crum <ad...@hlmksw.com>.
As I mentioned in another email, I was just making an observation. It's food for 
thought.

The zipped pages browser setting doesn't address the fundamental issue I 
presented: OFBiz servers are pushing out a lot of unnecessary markup.

It would be interesting to try out an OFBiz installation where 
Freemarker/Tomcat/whatever is set up to compress ALL markup, then see how much 
more responsive the web site is.


David E. Jones wrote:
> 
> The best way I've seen to handle this sort of thing is to take  
> advantage of the fact that pretty much all browsers support zipped  
> pages. I haven't set this sort of thing up in a LONG time, but there  
> are probably ways to do it with Tomcat, and definitely ways to do it  
> with the Apache web server (httpd).
> 
> -David
> 
> 
> On Jan 18, 2007, at 1:40 PM, Adrian Crum wrote:
> 
>> Just for grins, I inserted <#compress> </#compress> FTL directives  in 
>> the Party Manager FTL files to see how much smaller the markup  would 
>> be. Results:
>>
>> Before compress - 45k
>> After compress - 35k
>> 33% less markup.
>>
>> The drawback is, some of the layout seems to depend on some of the  
>> FTL whitespace, so the page's appearance changed a little.
>>
>>
>> Adrian Crum wrote:
>>
>>> After spending some time examining the unintentional formatting  
>>> changes in my patch files, I discovered that my editor  automatically 
>>> strips off unnecessary white space at the end of  every line. I can't 
>>> find a way to shut it off, so I'll have to  switch to another IDE.
>>> At first I was upset that my editor would do such a thing without  my 
>>> permission. Then I got to thinking that it makes a lot of  sense. 
>>> Less unnecessary white space equals less fluff the compiler  has to 
>>> trudge through and less fluff in HTML code.
>>> Hey! Wait a second... many of those files that were  unintentionally 
>>> formatted were FTL files. Does that mean that  OFBiz servers are 
>>> spewing out unnecessary fluff? I viewed the page  source on a typical 
>>> OFBiz web page and sure enough - OFBiz's  markup has unnecessary 
>>> white space at the end of the lines.
>>> Going through all of the FTL files and cleaning them up would be  
>>> easy to do with  a script or something, but the reduction in HTML  
>>> output would be small. Where I see a huge amount of unnecessary  
>>> markup is with indentation. Our four character indentation rule  
>>> results in things like a simple </div> tag being preceded by  twelve 
>>> to sixteen space characters. Our servers are working very  hard to 
>>> output nicely indented markup.
> 
> 

Re: OFBiz UI work, Plus New Revelations

Posted by "David E. Jones" <jo...@hotwaxmedia.com>.
The best way I've seen to handle this sort of thing is to take  
advantage of the fact that pretty much all browsers support zipped  
pages. I haven't set this sort of thing up in a LONG time, but there  
are probably ways to do it with Tomcat, and definitely ways to do it  
with the Apache web server (httpd).

-David


On Jan 18, 2007, at 1:40 PM, Adrian Crum wrote:

> Just for grins, I inserted <#compress> </#compress> FTL directives  
> in the Party Manager FTL files to see how much smaller the markup  
> would be. Results:
>
> Before compress - 45k
> After compress - 35k
> 33% less markup.
>
> The drawback is, some of the layout seems to depend on some of the  
> FTL whitespace, so the page's appearance changed a little.
>
>
> Adrian Crum wrote:
>
>> After spending some time examining the unintentional formatting  
>> changes in my patch files, I discovered that my editor  
>> automatically strips off unnecessary white space at the end of  
>> every line. I can't find a way to shut it off, so I'll have to  
>> switch to another IDE.
>> At first I was upset that my editor would do such a thing without  
>> my permission. Then I got to thinking that it makes a lot of  
>> sense. Less unnecessary white space equals less fluff the compiler  
>> has to trudge through and less fluff in HTML code.
>> Hey! Wait a second... many of those files that were  
>> unintentionally formatted were FTL files. Does that mean that  
>> OFBiz servers are spewing out unnecessary fluff? I viewed the page  
>> source on a typical OFBiz web page and sure enough - OFBiz's  
>> markup has unnecessary white space at the end of the lines.
>> Going through all of the FTL files and cleaning them up would be  
>> easy to do with  a script or something, but the reduction in HTML  
>> output would be small. Where I see a huge amount of unnecessary  
>> markup is with indentation. Our four character indentation rule  
>> results in things like a simple </div> tag being preceded by  
>> twelve to sixteen space characters. Our servers are working very  
>> hard to output nicely indented markup.


Re: OFBiz UI work, Plus New Revelations

Posted by Adrian Crum <ad...@hlmksw.com>.
Just for grins, I inserted <#compress> </#compress> FTL directives in the Party 
Manager FTL files to see how much smaller the markup would be. Results:

Before compress - 45k
After compress - 35k
33% less markup.

The drawback is, some of the layout seems to depend on some of the FTL 
whitespace, so the page's appearance changed a little.


Adrian Crum wrote:

> After spending some time examining the unintentional formatting changes 
> in my patch files, I discovered that my editor automatically strips off 
> unnecessary white space at the end of every line. I can't find a way to 
> shut it off, so I'll have to switch to another IDE.
> 
> At first I was upset that my editor would do such a thing without my 
> permission. Then I got to thinking that it makes a lot of sense. Less 
> unnecessary white space equals less fluff the compiler has to trudge 
> through and less fluff in HTML code.
> 
> Hey! Wait a second... many of those files that were unintentionally 
> formatted were FTL files. Does that mean that OFBiz servers are spewing 
> out unnecessary fluff? I viewed the page source on a typical OFBiz web 
> page and sure enough - OFBiz's markup has unnecessary white space at the 
> end of the lines.
> 
> Going through all of the FTL files and cleaning them up would be easy to 
> do with  a script or something, but the reduction in HTML output would 
> be small. Where I see a huge amount of unnecessary markup is with 
> indentation. Our four character indentation rule results in things like 
> a simple </div> tag being preceded by twelve to sixteen space 
> characters. Our servers are working very hard to output nicely indented 
> markup.
> 
> 

Re: OFBiz UI work, Plus New Revelations

Posted by Adrian Crum <ad...@hlmksw.com>.
After spending some time examining the unintentional formatting changes in my 
patch files, I discovered that my editor automatically strips off unnecessary 
white space at the end of every line. I can't find a way to shut it off, so I'll 
have to switch to another IDE.

At first I was upset that my editor would do such a thing without my permission. 
Then I got to thinking that it makes a lot of sense. Less unnecessary white 
space equals less fluff the compiler has to trudge through and less fluff in 
HTML code.

Hey! Wait a second... many of those files that were unintentionally formatted 
were FTL files. Does that mean that OFBiz servers are spewing out unnecessary 
fluff? I viewed the page source on a typical OFBiz web page and sure enough - 
OFBiz's markup has unnecessary white space at the end of the lines.

Going through all of the FTL files and cleaning them up would be easy to do with 
  a script or something, but the reduction in HTML output would be small. Where 
I see a huge amount of unnecessary markup is with indentation. Our four 
character indentation rule results in things like a simple </div> tag being 
preceded by twelve to sixteen space characters. Our servers are working very 
hard to output nicely indented markup.


Re: OFBiz UI work

Posted by Adrian Crum <ad...@hlmksw.com>.
Wow. This subject sure took off from my original dilemma.

Actually, there's an element of truth to what Chris said. At first glance the 
policy seems mean-spirited. But as was mentioned in another reply, it's a good 
way to train the submitters.

Personally, I have a pretty thick skin, so what David proposed doesn't offend me 
or discourage me. Instead, it motivates me to take a closer look at what my IDE 
and SVN client are doing to cause these unintentional formatting changes.

I truly appreciate the time David took to address my problem. Without his 
detailed description of the problems with my patches, I wouldn't know what was 
wrong with them.


David E. Jones wrote:
> 
> I'm not sure what to say, perhaps what I originally said would be  helpful:
> 
> "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
> 
> 
> On Jan 15, 2007, at 10:44 PM, Chris Howe wrote:
> 
>> Sorry, I don't know why that quote didn't hit me right
>> the first time.  My apologies to W.C. Fields. “Go
>> away, kid, you bother me”. I really butchered it. I
>> mean _really :-)
>>
>> So, if that's not the policy and procedure you're
>> looking to adopt, please elaborate on what you're
>> tempted to propose.
>>
>>
>> --- "David E. Jones" <jo...@hotwaxmedia.com> wrote:
>>
>>>
>>> You have quite an imagination.
>>>
>>> -David
>>>
>>>
>>> On Jan 15, 2007, at 10:01 PM, Chris Howe wrote:
>>>
>>>> Being tempted to ask for a vote, insinuates that
>>>
>>> you
>>>
>>>> want to condone a practice whereby if you go to
>>>
>>> review
>>>
>>>> a patch like in Adrian's case and it has
>>>
>>> superflous
>>>
>>>> formatting changes, that you would delete it from
>>>
>>> JIRA
>>>
>>>> in a "sorry kid, you're wasting my time" dismissal
>>>> (email impressions added for levity).  Anything
>>>
>>> less
>>>
>>>> is what is currently going on and wouldn't need a
>>>> vote.  People aren't tempted to vote on the status
>>>> quo.
>>>>
>>>> --- "David E. Jones" <jo...@hotwaxmedia.com>
>>>
>>> wrote:
>>>
>>>>
>>>>>
>>>>> Ummm... what do you think when you think of a
>>>>> rejection policy? We
>>>>> already have patch rejection policies when
>>>
>>> problems
>>>
>>>>> are found...
>>>>>
>>>>> -David
>>>>>
>>>>>
>>>>> On Jan 15, 2007, at 9:44 PM, Chris Howe wrote:
>>>>>
>>>>>> I agree.  I think it's better that we strongly
>>>>>> encourage certain practices (as we are doing
>>>
>>> now)
>>>
>>>>> and
>>>>>
>>>>>> bring people to notice when those practices
>>>
>>> could
>>>
>>>>> be
>>>>>
>>>>>> improved, but rejection policies put your _means
>>>>>> perspective ahead of your _ends.  Contributions
>>>>>
>>>>> may be
>>>>>
>>>>>> easier to review, but I can gaurantee you with
>>>>>> rejection policies, you will then have less of
>>>
>>> it
>>>
>>>>> to
>>>>>
>>>>>> review and thereby less solutions making it back
>>>>>
>>>>> into
>>>>>
>>>>>> the project.
>>>>>>
>>>>>> --- "David E. Jones" <jo...@hotwaxmedia.com>
>>>>>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Yes, we want more people, but I don't think
>>>>>
>>>>> anyone
>>>>>
>>>>>>> wants
>>>>>>> indiscriminate changes going into SVN! I hate
>>>>>>> surprises when I check
>>>>>>> out as much as the next guy, probably more than
>>>>>
>>>>> the
>>>>>
>>>>>>> next guy in many
>>>>>>> cases.
>>>>>>>
>>>>>>> Thinking about the next guy is the real key
>>>
>>> here.
>>>
>>>>>>> You may want to get
>>>>>>> your patches in ASAP, but if your patch breaks
>>>>>>> existing code (for
>>>>>>> example), then that needs to be fixed before
>>>
>>> the
>>>
>>>>>>> commit is done
>>>>>>> (either by the committer, the contributor, or
>>>
>>> an
>>>
>>>>>>> interested third
>>>>>>> party).
>>>>>>>
>>>>>>> So, yeah, that slows things down and is
>>>>>>> inconvenient, but keep in
>>>>>>> mind that a large portion of patches that come
>>>>>
>>>>> into
>>>>>
>>>>>>> OFBiz break
>>>>>>> existing functionality. This seems to happen
>>>>>
>>>>> around
>>>>>
>>>>>>> once a week or
>>>>>>> so, at least.
>>>>>>>
>>>>>>> It's great that people want to contribute, but
>>>
>>> in
>>>
>>>>> or
>>>>>
>>>>>>> to contribute to
>>>>>>> something as large and complex as OFBiz a large
>>>>>>> amount of effort is
>>>>>>> necessary, and anyone that wants to help out
>>>
>>> will
>>>
>>>>>>> err on the side of
>>>>>>> extra effort, caution and review, and
>>>>>
>>>>> consideration
>>>>>
>>>>>>> of more general
>>>>>>> requirements and designs rather just one's own
>>>>>>> requirements.
>>>>>>>
>>>>>>> -David
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Jan 15, 2007, at 7:42 PM, Jonathon --
>>>
>>> Improov
>>>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Heh. True. I definitely want MORE people
>>>>>
>>>>> involved
>>>>>
>>>>>>> in OFBiz.
>>>>>>>
>>>>>>>>
>>>>>>>> But since I'm not a committer, I'd rather make
>>>>>>>
>>>>>>> things easier for
>>>>>>>
>>>>>>>> the committers. I have no control over how
>>>
>>> easy
>>>
>>>>> or
>>>>>
>>>>>>> difficult it is
>>>>>>>
>>>>>>>> to handle patches with "extra unintended
>>>>>>>
>>>>>>> footprints".
>>>>>>>
>>>>>>>>
>>>>>>>> If I were a committer, I would try to
>>>>>>>
>>>>>>> automatically filter out
>>>>>>>
>>>>>>>> formatting changes in my audit, and then
>>>
>>> INCLUDE
>>>
>>>>>>> the formatting
>>>>>>>
>>>>>>>> changes. After all, there's no harm removing
>>>>>
>>>>> some
>>>>>
>>>>>>> extra spaces at
>>>>>>>
>>>>>>>> the end of lines, part of clean up.
>>>>>>>>
>>>>>>>> We often try to make things easier for the
>>>>>
>>>>> person
>>>>>
>>>>>>> who is doing a
>>>>>>>
>>>>>>>> task that we have no control over. Eg, I'd
>>>
>>> keep
>>>
>>>>> my
>>>>>
>>>>>>> mouth really
>>>>>>>
>>>>>>>> wide open for my dentist just so his vision is
>>>>>>>
>>>>>>> 20/20, no arguments
>>>>>>>
>>>>>>>> from me; I could afford to be slack about this
>>>>>>>
>>>>>>> "mouth wide-open
>>>>>>>
>>>>>>>> policy" if I were able to do the dentistry
>>>>>
>>>>> myself.
>>>>>
>>>>>>>>
>>>>>>>> But you're certainly right that it'll exclude
>>>>>
>>>>> some
>>>>>
>>>>>>> people,
>>>>>>>
>>>>>>>> especially folks who use editors that do not
>>>>>
>>>>> give
>>>>>
>>>>>>> very much control
>>>>>>>
>>>>>>>> to users.
>>>>>>>>
>>>>>>>> Jonathon
>>>>>>>>
>>>>>>>> Chris Howe wrote:
>>>>>>>>
>>>>>>>>> I don't know that rejection policies are very
>>>>>>>>> community friendly.  Treating SVN code
>>>
>>> changes
>>>
>>>>>>> like
>>>>>>>
>>>>>>>>> the keeper of the Bridge of Death (Monty
>>>
>>> Python
>>>
>>>>>>>>> Reference, smile) may find less people
>>>
>>> willing
>>>
>>>>> to
>>>>>
>>>>>>> do
>>>>>>>
>>>>>>>>> in this do-ocracy.  Many of us rather like
>>>>>
>>>>> what's
>>>>>
>>>>>>> left
>>>>>>>
>>>>>>>>> of our anarco-sydicalist commune (oh look,
>>>>>>>
>>>>>>> there's
>>>>>>>
>>>>>>>>> another :-) ).
>>>>>>>>> --- Jonathon -- Improov <jo...@improov.com>
>>>>>
>>>>> wrote:
>>>>>
>>>>>>>>>> David,
>>>>>>>>>>
>>>>>>>>>> I agree with the rejection policy.
>>>>>>>>>>
>>>>>>>>>> One suggestion on procedure to take when
>>>>>>>>>> self-reviewing a patch before submitting it.
>>>>>>>
>>>>>>> Look at
>>>>>>>
>>>>>>>>>> the diff to see if there are changes we
>>>
>>> cannot
>>>
>>>>>>> account
>>>>>>>
>>>>>>>>>> for. Using KDiff also makes things easier,
>>>>>
>>>>> even
>>>>>
>>>>>>> when dealing with
>>>>>>>
>>>>>>>>>> formatting changes.
>>>>>>>>>>
>>>>>>>>>> In Emacs, it's also easy to go through every
>>>>>
>>>>> set
>>>>>
>>>>>>> of
>>>>>>>
>>>>>>>>>> changes, do an interactive-search for 1st
>>>
>>>
>> === message truncated ===
>>
> 

Re: OFBiz UI work

Posted by Andrew Sykes <an...@sykesdevelopment.com>.
Adrian,

Too true, I suspect if David was paid standard rate for all the time
he's spent explaining what's wrong with patches he'd probably have
retired by now!

Being thick skinned is all part of working with a disparate group of
people on a very generic project. So perhaps a rejection policy also
helps people to understand the type of mindset required for success!

- Andrew


On Tue, 2007-01-16 at 07:56 -0800, Adrian Crum wrote:
> Wow. This subject sure took off from my original dilemma.
> 
> Actually, there's an element of truth to what Chris said. At first glance the 
> policy seems mean-spirited. But as was mentioned in another reply, it's a good 
> way to train the submitters.
> 
> Personally, I have a pretty thick skin, so what David proposed doesn't offend me 
> or discourage me. Instead, it motivates me to take a closer look at what my IDE 
> and SVN client are doing to cause these unintentional formatting changes.
> 
> I truly appreciate the time David took to address my problem. Without his 
> detailed description of the problems with my patches, I wouldn't know what was 
> wrong with them.
> 
> 
> David E. Jones wrote:
> > 
> > I'm not sure what to say, perhaps what I originally said would be  helpful:
> > 
> > "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
> > 
> > 
> > On Jan 15, 2007, at 10:44 PM, Chris Howe wrote:
> > 
> >> Sorry, I don't know why that quote didn't hit me right
> >> the first time.  My apologies to W.C. Fields. “Go
> >> away, kid, you bother me”. I really butchered it. I
> >> mean _really :-)
> >>
> >> So, if that's not the policy and procedure you're
> >> looking to adopt, please elaborate on what you're
> >> tempted to propose.
> >>
> >>
> >> --- "David E. Jones" <jo...@hotwaxmedia.com> wrote:
> >>
> >>>
> >>> You have quite an imagination.
> >>>
> >>> -David
> >>>
> >>>
> >>> On Jan 15, 2007, at 10:01 PM, Chris Howe wrote:
> >>>
> >>>> Being tempted to ask for a vote, insinuates that
> >>>
> >>> you
> >>>
> >>>> want to condone a practice whereby if you go to
> >>>
> >>> review
> >>>
> >>>> a patch like in Adrian's case and it has
> >>>
> >>> superflous
> >>>
> >>>> formatting changes, that you would delete it from
> >>>
> >>> JIRA
> >>>
> >>>> in a "sorry kid, you're wasting my time" dismissal
> >>>> (email impressions added for levity).  Anything
> >>>
> >>> less
> >>>
> >>>> is what is currently going on and wouldn't need a
> >>>> vote.  People aren't tempted to vote on the status
> >>>> quo.
> >>>>
> >>>> --- "David E. Jones" <jo...@hotwaxmedia.com>
> >>>
> >>> wrote:
> >>>
> >>>>
> >>>>>
> >>>>> Ummm... what do you think when you think of a
> >>>>> rejection policy? We
> >>>>> already have patch rejection policies when
> >>>
> >>> problems
> >>>
> >>>>> are found...
> >>>>>
> >>>>> -David
> >>>>>
> >>>>>
> >>>>> On Jan 15, 2007, at 9:44 PM, Chris Howe wrote:
> >>>>>
> >>>>>> I agree.  I think it's better that we strongly
> >>>>>> encourage certain practices (as we are doing
> >>>
> >>> now)
> >>>
> >>>>> and
> >>>>>
> >>>>>> bring people to notice when those practices
> >>>
> >>> could
> >>>
> >>>>> be
> >>>>>
> >>>>>> improved, but rejection policies put your _means
> >>>>>> perspective ahead of your _ends.  Contributions
> >>>>>
> >>>>> may be
> >>>>>
> >>>>>> easier to review, but I can gaurantee you with
> >>>>>> rejection policies, you will then have less of
> >>>
> >>> it
> >>>
> >>>>> to
> >>>>>
> >>>>>> review and thereby less solutions making it back
> >>>>>
> >>>>> into
> >>>>>
> >>>>>> the project.
> >>>>>>
> >>>>>> --- "David E. Jones" <jo...@hotwaxmedia.com>
> >>>>>
> >>>>> wrote:
> >>>>>
> >>>>>>
> >>>>>>>
> >>>>>>> Yes, we want more people, but I don't think
> >>>>>
> >>>>> anyone
> >>>>>
> >>>>>>> wants
> >>>>>>> indiscriminate changes going into SVN! I hate
> >>>>>>> surprises when I check
> >>>>>>> out as much as the next guy, probably more than
> >>>>>
> >>>>> the
> >>>>>
> >>>>>>> next guy in many
> >>>>>>> cases.
> >>>>>>>
> >>>>>>> Thinking about the next guy is the real key
> >>>
> >>> here.
> >>>
> >>>>>>> You may want to get
> >>>>>>> your patches in ASAP, but if your patch breaks
> >>>>>>> existing code (for
> >>>>>>> example), then that needs to be fixed before
> >>>
> >>> the
> >>>
> >>>>>>> commit is done
> >>>>>>> (either by the committer, the contributor, or
> >>>
> >>> an
> >>>
> >>>>>>> interested third
> >>>>>>> party).
> >>>>>>>
> >>>>>>> So, yeah, that slows things down and is
> >>>>>>> inconvenient, but keep in
> >>>>>>> mind that a large portion of patches that come
> >>>>>
> >>>>> into
> >>>>>
> >>>>>>> OFBiz break
> >>>>>>> existing functionality. This seems to happen
> >>>>>
> >>>>> around
> >>>>>
> >>>>>>> once a week or
> >>>>>>> so, at least.
> >>>>>>>
> >>>>>>> It's great that people want to contribute, but
> >>>
> >>> in
> >>>
> >>>>> or
> >>>>>
> >>>>>>> to contribute to
> >>>>>>> something as large and complex as OFBiz a large
> >>>>>>> amount of effort is
> >>>>>>> necessary, and anyone that wants to help out
> >>>
> >>> will
> >>>
> >>>>>>> err on the side of
> >>>>>>> extra effort, caution and review, and
> >>>>>
> >>>>> consideration
> >>>>>
> >>>>>>> of more general
> >>>>>>> requirements and designs rather just one's own
> >>>>>>> requirements.
> >>>>>>>
> >>>>>>> -David
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> On Jan 15, 2007, at 7:42 PM, Jonathon --
> >>>
> >>> Improov
> >>>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> Heh. True. I definitely want MORE people
> >>>>>
> >>>>> involved
> >>>>>
> >>>>>>> in OFBiz.
> >>>>>>>
> >>>>>>>>
> >>>>>>>> But since I'm not a committer, I'd rather make
> >>>>>>>
> >>>>>>> things easier for
> >>>>>>>
> >>>>>>>> the committers. I have no control over how
> >>>
> >>> easy
> >>>
> >>>>> or
> >>>>>
> >>>>>>> difficult it is
> >>>>>>>
> >>>>>>>> to handle patches with "extra unintended
> >>>>>>>
> >>>>>>> footprints".
> >>>>>>>
> >>>>>>>>
> >>>>>>>> If I were a committer, I would try to
> >>>>>>>
> >>>>>>> automatically filter out
> >>>>>>>
> >>>>>>>> formatting changes in my audit, and then
> >>>
> >>> INCLUDE
> >>>
> >>>>>>> the formatting
> >>>>>>>
> >>>>>>>> changes. After all, there's no harm removing
> >>>>>
> >>>>> some
> >>>>>
> >>>>>>> extra spaces at
> >>>>>>>
> >>>>>>>> the end of lines, part of clean up.
> >>>>>>>>
> >>>>>>>> We often try to make things easier for the
> >>>>>
> >>>>> person
> >>>>>
> >>>>>>> who is doing a
> >>>>>>>
> >>>>>>>> task that we have no control over. Eg, I'd
> >>>
> >>> keep
> >>>
> >>>>> my
> >>>>>
> >>>>>>> mouth really
> >>>>>>>
> >>>>>>>> wide open for my dentist just so his vision is
> >>>>>>>
> >>>>>>> 20/20, no arguments
> >>>>>>>
> >>>>>>>> from me; I could afford to be slack about this
> >>>>>>>
> >>>>>>> "mouth wide-open
> >>>>>>>
> >>>>>>>> policy" if I were able to do the dentistry
> >>>>>
> >>>>> myself.
> >>>>>
> >>>>>>>>
> >>>>>>>> But you're certainly right that it'll exclude
> >>>>>
> >>>>> some
> >>>>>
> >>>>>>> people,
> >>>>>>>
> >>>>>>>> especially folks who use editors that do not
> >>>>>
> >>>>> give
> >>>>>
> >>>>>>> very much control
> >>>>>>>
> >>>>>>>> to users.
> >>>>>>>>
> >>>>>>>> Jonathon
> >>>>>>>>
> >>>>>>>> Chris Howe wrote:
> >>>>>>>>
> >>>>>>>>> I don't know that rejection policies are very
> >>>>>>>>> community friendly.  Treating SVN code
> >>>
> >>> changes
> >>>
> >>>>>>> like
> >>>>>>>
> >>>>>>>>> the keeper of the Bridge of Death (Monty
> >>>
> >>> Python
> >>>
> >>>>>>>>> Reference, smile) may find less people
> >>>
> >>> willing
> >>>
> >>>>> to
> >>>>>
> >>>>>>> do
> >>>>>>>
> >>>>>>>>> in this do-ocracy.  Many of us rather like
> >>>>>
> >>>>> what's
> >>>>>
> >>>>>>> left
> >>>>>>>
> >>>>>>>>> of our anarco-sydicalist commune (oh look,
> >>>>>>>
> >>>>>>> there's
> >>>>>>>
> >>>>>>>>> another :-) ).
> >>>>>>>>> --- Jonathon -- Improov <jo...@improov.com>
> >>>>>
> >>>>> wrote:
> >>>>>
> >>>>>>>>>> David,
> >>>>>>>>>>
> >>>>>>>>>> I agree with the rejection policy.
> >>>>>>>>>>
> >>>>>>>>>> One suggestion on procedure to take when
> >>>>>>>>>> self-reviewing a patch before submitting it.
> >>>>>>>
> >>>>>>> Look at
> >>>>>>>
> >>>>>>>>>> the diff to see if there are changes we
> >>>
> >>> cannot
> >>>
> >>>>>>> account
> >>>>>>>
> >>>>>>>>>> for. Using KDiff also makes things easier,
> >>>>>
> >>>>> even
> >>>>>
> >>>>>>> when dealing with
> >>>>>>>
> >>>>>>>>>> formatting changes.
> >>>>>>>>>>
> >>>>>>>>>> In Emacs, it's also easy to go through every
> >>>>>
> >>>>> set
> >>>>>
> >>>>>>> of
> >>>>>>>
> >>>>>>>>>> changes, do an interactive-search for 1st
> >>>
> >>>
> >> === message truncated ===
> >>
> > 
-- 
Kind Regards
Andrew Sykes <an...@sykesdevelopment.com>
Sykes Development Ltd
http://www.sykesdevelopment.com


Re: OFBiz UI work

Posted by Chris Howe <cj...@yahoo.com>.
So, then how does the parodied scenario demonstrate an
imagination if that's exactly what you're wanting to
do? 

--- "David E. Jones" <jo...@hotwaxmedia.com> wrote:

> 
> I'm not sure what to say, perhaps what I originally
> said would be  
> helpful:
> 
> "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
> 
> 
> On Jan 15, 2007, at 10:44 PM, Chris Howe wrote:
> 
> > Sorry, I don't know why that quote didn't hit me
> right
> > the first time.  My apologies to W.C. Fields. “Go
> > away, kid, you bother me”. I really butchered it.
> I
> > mean _really :-)
> >
> > So, if that's not the policy and procedure you're
> > looking to adopt, please elaborate on what you're
> > tempted to propose.
> >
> >
> > --- "David E. Jones" <jo...@hotwaxmedia.com>
> wrote:
> >
> >>
> >> You have quite an imagination.
> >>
> >> -David
> >>
> >>
> >> On Jan 15, 2007, at 10:01 PM, Chris Howe wrote:
> >>
> >>> Being tempted to ask for a vote, insinuates that
> >> you
> >>> want to condone a practice whereby if you go to
> >> review
> >>> a patch like in Adrian's case and it has
> >> superflous
> >>> formatting changes, that you would delete it
> from
> >> JIRA
> >>> in a "sorry kid, you're wasting my time"
> dismissal
> >>> (email impressions added for levity).  Anything
> >> less
> >>> is what is currently going on and wouldn't need
> a
> >>> vote.  People aren't tempted to vote on the
> status
> >>> quo.
> >>>
> >>> --- "David E. Jones" <jo...@hotwaxmedia.com>
> >> wrote:
> >>>
> >>>>
> >>>> Ummm... what do you think when you think of a
> >>>> rejection policy? We
> >>>> already have patch rejection policies when
> >> problems
> >>>> are found...
> >>>>
> >>>> -David
> >>>>
> >>>>
> >>>> On Jan 15, 2007, at 9:44 PM, Chris Howe wrote:
> >>>>
> >>>>> I agree.  I think it's better that we strongly
> >>>>> encourage certain practices (as we are doing
> >> now)
> >>>> and
> >>>>> bring people to notice when those practices
> >> could
> >>>> be
> >>>>> improved, but rejection policies put your
> _means
> >>>>> perspective ahead of your _ends. 
> Contributions
> >>>> may be
> >>>>> easier to review, but I can gaurantee you with
> >>>>> rejection policies, you will then have less of
> >> it
> >>>> to
> >>>>> review and thereby less solutions making it
> back
> >>>> into
> >>>>> the project.
> >>>>>
> >>>>> --- "David E. Jones" <jo...@hotwaxmedia.com>
> >>>> wrote:
> >>>>>
> >>>>>>
> >>>>>> Yes, we want more people, but I don't think
> >>>> anyone
> >>>>>> wants
> >>>>>> indiscriminate changes going into SVN! I hate
> >>>>>> surprises when I check
> >>>>>> out as much as the next guy, probably more
> than
> >>>> the
> >>>>>> next guy in many
> >>>>>> cases.
> >>>>>>
> >>>>>> Thinking about the next guy is the real key
> >> here.
> >>>>>> You may want to get
> >>>>>> your patches in ASAP, but if your patch
> breaks
> >>>>>> existing code (for
> >>>>>> example), then that needs to be fixed before
> >> the
> >>>>>> commit is done
> >>>>>> (either by the committer, the contributor, or
> >> an
> >>>>>> interested third
> >>>>>> party).
> >>>>>>
> >>>>>> So, yeah, that slows things down and is
> >>>>>> inconvenient, but keep in
> >>>>>> mind that a large portion of patches that
> come
> >>>> into
> >>>>>> OFBiz break
> >>>>>> existing functionality. This seems to happen
> >>>> around
> >>>>>> once a week or
> >>>>>> so, at least.
> >>>>>>
> >>>>>> It's great that people want to contribute,
> but
> >> in
> >>>> or
> >>>>>> to contribute to
> >>>>>> something as large and complex as OFBiz a
> large
> >>>>>> amount of effort is
> >>>>>> necessary, and anyone that wants to help out
> >> will
> >>>>>> err on the side of
> >>>>>> extra effort, caution and review, and
> >>>> consideration
> >>>>>> of more general
> >>>>>> requirements and designs rather just one's
> own
> >>>>>> requirements.
> >>>>>>
> >>>>>> -David
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On Jan 15, 2007, at 7:42 PM, Jonathon --
> >> Improov
> >>>>>> wrote:
> >>>>>>
> >>>>>>> Heh. True. I definitely want MORE people
> >>>> involved
> >>>>>> in OFBiz.
> >>>>>>>
> >>>>>>> But since I'm not a committer, I'd rather
> make
> >>>>>> things easier for
> >>>>>>> the committers. I have no control over how
> >> easy
> >>>> or
> >>>>>> difficult it is
> >>>>>>> to handle patches with "extra unintended
> >>>>>> footprints".
> >>>>>>>
> >>>>>>> If I were a committer, I would try to
> >>>>>> automatically filter out
> >>>>>>> formatting changes in my audit, and then
> >> INCLUDE
> >>>>>> the formatting
> >>>>>>> changes. After all, there's no harm removing
> >>>> some
> >>>>>> extra spaces at
> >>>>>>> the end of lines, part of clean up.
> >>>>>>>
> >>>>>>> We often try to make things easier for the
> >>>> person
> >>>>>> who is doing a
> >>>>>>> task that we have no control over. Eg, I'd
> >> keep
> >>>> my
> >>>>>> mouth really
> >>>>>>> wide open for my dentist just so his vision
> is
> >>>>>> 20/20, no arguments
> >>>>>>> from me; I could afford to be slack about
> this
> >>>>>> "mouth wide-open
> >>>>>>> policy" if I were able to do the dentistry
> >>>> myself.
> 
=== message truncated ===


Re: OFBiz UI work

Posted by "David E. Jones" <jo...@hotwaxmedia.com>.
I'm not sure what to say, perhaps what I originally said would be  
helpful:

"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


On Jan 15, 2007, at 10:44 PM, Chris Howe wrote:

> Sorry, I don't know why that quote didn't hit me right
> the first time.  My apologies to W.C. Fields. “Go
> away, kid, you bother me”. I really butchered it. I
> mean _really :-)
>
> So, if that's not the policy and procedure you're
> looking to adopt, please elaborate on what you're
> tempted to propose.
>
>
> --- "David E. Jones" <jo...@hotwaxmedia.com> wrote:
>
>>
>> You have quite an imagination.
>>
>> -David
>>
>>
>> On Jan 15, 2007, at 10:01 PM, Chris Howe wrote:
>>
>>> Being tempted to ask for a vote, insinuates that
>> you
>>> want to condone a practice whereby if you go to
>> review
>>> a patch like in Adrian's case and it has
>> superflous
>>> formatting changes, that you would delete it from
>> JIRA
>>> in a "sorry kid, you're wasting my time" dismissal
>>> (email impressions added for levity).  Anything
>> less
>>> is what is currently going on and wouldn't need a
>>> vote.  People aren't tempted to vote on the status
>>> quo.
>>>
>>> --- "David E. Jones" <jo...@hotwaxmedia.com>
>> wrote:
>>>
>>>>
>>>> Ummm... what do you think when you think of a
>>>> rejection policy? We
>>>> already have patch rejection policies when
>> problems
>>>> are found...
>>>>
>>>> -David
>>>>
>>>>
>>>> On Jan 15, 2007, at 9:44 PM, Chris Howe wrote:
>>>>
>>>>> I agree.  I think it's better that we strongly
>>>>> encourage certain practices (as we are doing
>> now)
>>>> and
>>>>> bring people to notice when those practices
>> could
>>>> be
>>>>> improved, but rejection policies put your _means
>>>>> perspective ahead of your _ends.  Contributions
>>>> may be
>>>>> easier to review, but I can gaurantee you with
>>>>> rejection policies, you will then have less of
>> it
>>>> to
>>>>> review and thereby less solutions making it back
>>>> into
>>>>> the project.
>>>>>
>>>>> --- "David E. Jones" <jo...@hotwaxmedia.com>
>>>> wrote:
>>>>>
>>>>>>
>>>>>> Yes, we want more people, but I don't think
>>>> anyone
>>>>>> wants
>>>>>> indiscriminate changes going into SVN! I hate
>>>>>> surprises when I check
>>>>>> out as much as the next guy, probably more than
>>>> the
>>>>>> next guy in many
>>>>>> cases.
>>>>>>
>>>>>> Thinking about the next guy is the real key
>> here.
>>>>>> You may want to get
>>>>>> your patches in ASAP, but if your patch breaks
>>>>>> existing code (for
>>>>>> example), then that needs to be fixed before
>> the
>>>>>> commit is done
>>>>>> (either by the committer, the contributor, or
>> an
>>>>>> interested third
>>>>>> party).
>>>>>>
>>>>>> So, yeah, that slows things down and is
>>>>>> inconvenient, but keep in
>>>>>> mind that a large portion of patches that come
>>>> into
>>>>>> OFBiz break
>>>>>> existing functionality. This seems to happen
>>>> around
>>>>>> once a week or
>>>>>> so, at least.
>>>>>>
>>>>>> It's great that people want to contribute, but
>> in
>>>> or
>>>>>> to contribute to
>>>>>> something as large and complex as OFBiz a large
>>>>>> amount of effort is
>>>>>> necessary, and anyone that wants to help out
>> will
>>>>>> err on the side of
>>>>>> extra effort, caution and review, and
>>>> consideration
>>>>>> of more general
>>>>>> requirements and designs rather just one's own
>>>>>> requirements.
>>>>>>
>>>>>> -David
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Jan 15, 2007, at 7:42 PM, Jonathon --
>> Improov
>>>>>> wrote:
>>>>>>
>>>>>>> Heh. True. I definitely want MORE people
>>>> involved
>>>>>> in OFBiz.
>>>>>>>
>>>>>>> But since I'm not a committer, I'd rather make
>>>>>> things easier for
>>>>>>> the committers. I have no control over how
>> easy
>>>> or
>>>>>> difficult it is
>>>>>>> to handle patches with "extra unintended
>>>>>> footprints".
>>>>>>>
>>>>>>> If I were a committer, I would try to
>>>>>> automatically filter out
>>>>>>> formatting changes in my audit, and then
>> INCLUDE
>>>>>> the formatting
>>>>>>> changes. After all, there's no harm removing
>>>> some
>>>>>> extra spaces at
>>>>>>> the end of lines, part of clean up.
>>>>>>>
>>>>>>> We often try to make things easier for the
>>>> person
>>>>>> who is doing a
>>>>>>> task that we have no control over. Eg, I'd
>> keep
>>>> my
>>>>>> mouth really
>>>>>>> wide open for my dentist just so his vision is
>>>>>> 20/20, no arguments
>>>>>>> from me; I could afford to be slack about this
>>>>>> "mouth wide-open
>>>>>>> policy" if I were able to do the dentistry
>>>> myself.
>>>>>>>
>>>>>>> But you're certainly right that it'll exclude
>>>> some
>>>>>> people,
>>>>>>> especially folks who use editors that do not
>>>> give
>>>>>> very much control
>>>>>>> to users.
>>>>>>>
>>>>>>> Jonathon
>>>>>>>
>>>>>>> Chris Howe wrote:
>>>>>>>> I don't know that rejection policies are very
>>>>>>>> community friendly.  Treating SVN code
>> changes
>>>>>> like
>>>>>>>> the keeper of the Bridge of Death (Monty
>> Python
>>>>>>>> Reference, smile) may find less people
>> willing
>>>> to
>>>>>> do
>>>>>>>> in this do-ocracy.  Many of us rather like
>>>> what's
>>>>>> left
>>>>>>>> of our anarco-sydicalist commune (oh look,
>>>>>> there's
>>>>>>>> another :-) ).
>>>>>>>> --- Jonathon -- Improov <jo...@improov.com>
>>>> wrote:
>>>>>>>>> David,
>>>>>>>>>
>>>>>>>>> I agree with the rejection policy.
>>>>>>>>>
>>>>>>>>> One suggestion on procedure to take when
>>>>>>>>> self-reviewing a patch before submitting it.
>>>>>> Look at
>>>>>>>>> the diff to see if there are changes we
>> cannot
>>>>>> account
>>>>>>>>> for. Using KDiff also makes things easier,
>>>> even
>>>>>> when dealing with
>>>>>>>>> formatting changes.
>>>>>>>>>
>>>>>>>>> In Emacs, it's also easy to go through every
>>>> set
>>>>>> of
>>>>>>>>> changes, do an interactive-search for 1st
>>
> === message truncated ===
>


Re: OFBiz UI work

Posted by Chris Howe <cj...@yahoo.com>.
Sorry, I don't know why that quote didn't hit me right
the first time.  My apologies to W.C. Fields. “Go
away, kid, you bother me”. I really butchered it. I
mean _really :-)

So, if that's not the policy and procedure you're
looking to adopt, please elaborate on what you're
tempted to propose.


--- "David E. Jones" <jo...@hotwaxmedia.com> wrote:

> 
> You have quite an imagination.
> 
> -David
> 
> 
> On Jan 15, 2007, at 10:01 PM, Chris Howe wrote:
> 
> > Being tempted to ask for a vote, insinuates that
> you
> > want to condone a practice whereby if you go to
> review
> > a patch like in Adrian's case and it has
> superflous
> > formatting changes, that you would delete it from
> JIRA
> > in a "sorry kid, you're wasting my time" dismissal
> > (email impressions added for levity).  Anything
> less
> > is what is currently going on and wouldn't need a
> > vote.  People aren't tempted to vote on the status
> > quo.
> >
> > --- "David E. Jones" <jo...@hotwaxmedia.com>
> wrote:
> >
> >>
> >> Ummm... what do you think when you think of a
> >> rejection policy? We
> >> already have patch rejection policies when
> problems
> >> are found...
> >>
> >> -David
> >>
> >>
> >> On Jan 15, 2007, at 9:44 PM, Chris Howe wrote:
> >>
> >>> I agree.  I think it's better that we strongly
> >>> encourage certain practices (as we are doing
> now)
> >> and
> >>> bring people to notice when those practices
> could
> >> be
> >>> improved, but rejection policies put your _means
> >>> perspective ahead of your _ends.  Contributions
> >> may be
> >>> easier to review, but I can gaurantee you with
> >>> rejection policies, you will then have less of
> it
> >> to
> >>> review and thereby less solutions making it back
> >> into
> >>> the project.
> >>>
> >>> --- "David E. Jones" <jo...@hotwaxmedia.com>
> >> wrote:
> >>>
> >>>>
> >>>> Yes, we want more people, but I don't think
> >> anyone
> >>>> wants
> >>>> indiscriminate changes going into SVN! I hate
> >>>> surprises when I check
> >>>> out as much as the next guy, probably more than
> >> the
> >>>> next guy in many
> >>>> cases.
> >>>>
> >>>> Thinking about the next guy is the real key
> here.
> >>>> You may want to get
> >>>> your patches in ASAP, but if your patch breaks
> >>>> existing code (for
> >>>> example), then that needs to be fixed before
> the
> >>>> commit is done
> >>>> (either by the committer, the contributor, or
> an
> >>>> interested third
> >>>> party).
> >>>>
> >>>> So, yeah, that slows things down and is
> >>>> inconvenient, but keep in
> >>>> mind that a large portion of patches that come
> >> into
> >>>> OFBiz break
> >>>> existing functionality. This seems to happen
> >> around
> >>>> once a week or
> >>>> so, at least.
> >>>>
> >>>> It's great that people want to contribute, but
> in
> >> or
> >>>> to contribute to
> >>>> something as large and complex as OFBiz a large
> >>>> amount of effort is
> >>>> necessary, and anyone that wants to help out
> will
> >>>> err on the side of
> >>>> extra effort, caution and review, and
> >> consideration
> >>>> of more general
> >>>> requirements and designs rather just one's own
> >>>> requirements.
> >>>>
> >>>> -David
> >>>>
> >>>>
> >>>>
> >>>> On Jan 15, 2007, at 7:42 PM, Jonathon --
> Improov
> >>>> wrote:
> >>>>
> >>>>> Heh. True. I definitely want MORE people
> >> involved
> >>>> in OFBiz.
> >>>>>
> >>>>> But since I'm not a committer, I'd rather make
> >>>> things easier for
> >>>>> the committers. I have no control over how
> easy
> >> or
> >>>> difficult it is
> >>>>> to handle patches with "extra unintended
> >>>> footprints".
> >>>>>
> >>>>> If I were a committer, I would try to
> >>>> automatically filter out
> >>>>> formatting changes in my audit, and then
> INCLUDE
> >>>> the formatting
> >>>>> changes. After all, there's no harm removing
> >> some
> >>>> extra spaces at
> >>>>> the end of lines, part of clean up.
> >>>>>
> >>>>> We often try to make things easier for the
> >> person
> >>>> who is doing a
> >>>>> task that we have no control over. Eg, I'd
> keep
> >> my
> >>>> mouth really
> >>>>> wide open for my dentist just so his vision is
> >>>> 20/20, no arguments
> >>>>> from me; I could afford to be slack about this
> >>>> "mouth wide-open
> >>>>> policy" if I were able to do the dentistry
> >> myself.
> >>>>>
> >>>>> But you're certainly right that it'll exclude
> >> some
> >>>> people,
> >>>>> especially folks who use editors that do not
> >> give
> >>>> very much control
> >>>>> to users.
> >>>>>
> >>>>> Jonathon
> >>>>>
> >>>>> Chris Howe wrote:
> >>>>>> I don't know that rejection policies are very
> >>>>>> community friendly.  Treating SVN code
> changes
> >>>> like
> >>>>>> the keeper of the Bridge of Death (Monty
> Python
> >>>>>> Reference, smile) may find less people
> willing
> >> to
> >>>> do
> >>>>>> in this do-ocracy.  Many of us rather like
> >> what's
> >>>> left
> >>>>>> of our anarco-sydicalist commune (oh look,
> >>>> there's
> >>>>>> another :-) ).
> >>>>>> --- Jonathon -- Improov <jo...@improov.com>
> >> wrote:
> >>>>>>> David,
> >>>>>>>
> >>>>>>> I agree with the rejection policy.
> >>>>>>>
> >>>>>>> One suggestion on procedure to take when
> >>>>>>> self-reviewing a patch before submitting it.
> >>>> Look at
> >>>>>>> the diff to see if there are changes we
> cannot
> >>>> account
> >>>>>>> for. Using KDiff also makes things easier,
> >> even
> >>>> when dealing with
> >>>>>>> formatting changes.
> >>>>>>>
> >>>>>>> In Emacs, it's also easy to go through every
> >> set
> >>>> of
> >>>>>>> changes, do an interactive-search for 1st
> 
=== message truncated ===


Re: OFBiz UI work

Posted by "David E. Jones" <jo...@hotwaxmedia.com>.
You have quite an imagination.

-David


On Jan 15, 2007, at 10:01 PM, Chris Howe wrote:

> Being tempted to ask for a vote, insinuates that you
> want to condone a practice whereby if you go to review
> a patch like in Adrian's case and it has superflous
> formatting changes, that you would delete it from JIRA
> in a "sorry kid, you're wasting my time" dismissal
> (email impressions added for levity).  Anything less
> is what is currently going on and wouldn't need a
> vote.  People aren't tempted to vote on the status
> quo.
>
> --- "David E. Jones" <jo...@hotwaxmedia.com> wrote:
>
>>
>> Ummm... what do you think when you think of a
>> rejection policy? We
>> already have patch rejection policies when problems
>> are found...
>>
>> -David
>>
>>
>> On Jan 15, 2007, at 9:44 PM, Chris Howe wrote:
>>
>>> I agree.  I think it's better that we strongly
>>> encourage certain practices (as we are doing now)
>> and
>>> bring people to notice when those practices could
>> be
>>> improved, but rejection policies put your _means
>>> perspective ahead of your _ends.  Contributions
>> may be
>>> easier to review, but I can gaurantee you with
>>> rejection policies, you will then have less of it
>> to
>>> review and thereby less solutions making it back
>> into
>>> the project.
>>>
>>> --- "David E. Jones" <jo...@hotwaxmedia.com>
>> wrote:
>>>
>>>>
>>>> Yes, we want more people, but I don't think
>> anyone
>>>> wants
>>>> indiscriminate changes going into SVN! I hate
>>>> surprises when I check
>>>> out as much as the next guy, probably more than
>> the
>>>> next guy in many
>>>> cases.
>>>>
>>>> Thinking about the next guy is the real key here.
>>>> You may want to get
>>>> your patches in ASAP, but if your patch breaks
>>>> existing code (for
>>>> example), then that needs to be fixed before the
>>>> commit is done
>>>> (either by the committer, the contributor, or an
>>>> interested third
>>>> party).
>>>>
>>>> So, yeah, that slows things down and is
>>>> inconvenient, but keep in
>>>> mind that a large portion of patches that come
>> into
>>>> OFBiz break
>>>> existing functionality. This seems to happen
>> around
>>>> once a week or
>>>> so, at least.
>>>>
>>>> It's great that people want to contribute, but in
>> or
>>>> to contribute to
>>>> something as large and complex as OFBiz a large
>>>> amount of effort is
>>>> necessary, and anyone that wants to help out will
>>>> err on the side of
>>>> extra effort, caution and review, and
>> consideration
>>>> of more general
>>>> requirements and designs rather just one's own
>>>> requirements.
>>>>
>>>> -David
>>>>
>>>>
>>>>
>>>> On Jan 15, 2007, at 7:42 PM, Jonathon -- Improov
>>>> wrote:
>>>>
>>>>> Heh. True. I definitely want MORE people
>> involved
>>>> in OFBiz.
>>>>>
>>>>> But since I'm not a committer, I'd rather make
>>>> things easier for
>>>>> the committers. I have no control over how easy
>> or
>>>> difficult it is
>>>>> to handle patches with "extra unintended
>>>> footprints".
>>>>>
>>>>> If I were a committer, I would try to
>>>> automatically filter out
>>>>> formatting changes in my audit, and then INCLUDE
>>>> the formatting
>>>>> changes. After all, there's no harm removing
>> some
>>>> extra spaces at
>>>>> the end of lines, part of clean up.
>>>>>
>>>>> We often try to make things easier for the
>> person
>>>> who is doing a
>>>>> task that we have no control over. Eg, I'd keep
>> my
>>>> mouth really
>>>>> wide open for my dentist just so his vision is
>>>> 20/20, no arguments
>>>>> from me; I could afford to be slack about this
>>>> "mouth wide-open
>>>>> policy" if I were able to do the dentistry
>> myself.
>>>>>
>>>>> But you're certainly right that it'll exclude
>> some
>>>> people,
>>>>> especially folks who use editors that do not
>> give
>>>> very much control
>>>>> to users.
>>>>>
>>>>> Jonathon
>>>>>
>>>>> Chris Howe wrote:
>>>>>> I don't know that rejection policies are very
>>>>>> community friendly.  Treating SVN code changes
>>>> like
>>>>>> the keeper of the Bridge of Death (Monty Python
>>>>>> Reference, smile) may find less people willing
>> to
>>>> do
>>>>>> in this do-ocracy.  Many of us rather like
>> what's
>>>> left
>>>>>> of our anarco-sydicalist commune (oh look,
>>>> there's
>>>>>> another :-) ).
>>>>>> --- Jonathon -- Improov <jo...@improov.com>
>> wrote:
>>>>>>> David,
>>>>>>>
>>>>>>> I agree with the rejection policy.
>>>>>>>
>>>>>>> One suggestion on procedure to take when
>>>>>>> self-reviewing a patch before submitting it.
>>>> Look at
>>>>>>> the diff to see if there are changes we cannot
>>>> account
>>>>>>> for. Using KDiff also makes things easier,
>> even
>>>> when dealing with
>>>>>>> formatting changes.
>>>>>>>
>>>>>>> In Emacs, it's also easy to go through every
>> set
>>>> of
>>>>>>> changes, do an interactive-search for 1st
>> entry
>>>> of each set, and
>>>>>>> see if the 2nd entry is
>>>>>>> highlighted similarly. Meaning, if it is
>>>> highlighted similarly,
>>>>>>> the set of changes is bogus
>>>>>>> (formatting only).
>>>>>>>
>>>>>>> In general, we should submit patches that are
>>>>>>> intentional, ie just the intended changes
>> only.
>>>> We should not
>>>>>>> submit patches that contain unintended
>>>>>>> changes that have extra unintended footprints.
>>>>>>>
>>>>>>> Jonathon
>>>>>>>
>>>>>>> David E. Jones wrote:
>>>>>>>> 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
>>
> === message truncated ===
>


Re: OFBiz UI work

Posted by Chris Howe <cj...@yahoo.com>.
Being tempted to ask for a vote, insinuates that you
want to condone a practice whereby if you go to review
a patch like in Adrian's case and it has superflous
formatting changes, that you would delete it from JIRA
in a "sorry kid, you're wasting my time" dismissal
(email impressions added for levity).  Anything less
is what is currently going on and wouldn't need a
vote.  People aren't tempted to vote on the status
quo.

--- "David E. Jones" <jo...@hotwaxmedia.com> wrote:

> 
> Ummm... what do you think when you think of a
> rejection policy? We  
> already have patch rejection policies when problems
> are found...
> 
> -David
> 
> 
> On Jan 15, 2007, at 9:44 PM, Chris Howe wrote:
> 
> > I agree.  I think it's better that we strongly
> > encourage certain practices (as we are doing now)
> and
> > bring people to notice when those practices could
> be
> > improved, but rejection policies put your _means
> > perspective ahead of your _ends.  Contributions
> may be
> > easier to review, but I can gaurantee you with
> > rejection policies, you will then have less of it
> to
> > review and thereby less solutions making it back
> into
> > the project.
> >
> > --- "David E. Jones" <jo...@hotwaxmedia.com>
> wrote:
> >
> >>
> >> Yes, we want more people, but I don't think
> anyone
> >> wants
> >> indiscriminate changes going into SVN! I hate
> >> surprises when I check
> >> out as much as the next guy, probably more than
> the
> >> next guy in many
> >> cases.
> >>
> >> Thinking about the next guy is the real key here.
> >> You may want to get
> >> your patches in ASAP, but if your patch breaks
> >> existing code (for
> >> example), then that needs to be fixed before the
> >> commit is done
> >> (either by the committer, the contributor, or an
> >> interested third
> >> party).
> >>
> >> So, yeah, that slows things down and is
> >> inconvenient, but keep in
> >> mind that a large portion of patches that come
> into
> >> OFBiz break
> >> existing functionality. This seems to happen
> around
> >> once a week or
> >> so, at least.
> >>
> >> It's great that people want to contribute, but in
> or
> >> to contribute to
> >> something as large and complex as OFBiz a large
> >> amount of effort is
> >> necessary, and anyone that wants to help out will
> >> err on the side of
> >> extra effort, caution and review, and
> consideration
> >> of more general
> >> requirements and designs rather just one's own
> >> requirements.
> >>
> >> -David
> >>
> >>
> >>
> >> On Jan 15, 2007, at 7:42 PM, Jonathon -- Improov
> >> wrote:
> >>
> >>> Heh. True. I definitely want MORE people
> involved
> >> in OFBiz.
> >>>
> >>> But since I'm not a committer, I'd rather make
> >> things easier for
> >>> the committers. I have no control over how easy
> or
> >> difficult it is
> >>> to handle patches with "extra unintended
> >> footprints".
> >>>
> >>> If I were a committer, I would try to
> >> automatically filter out
> >>> formatting changes in my audit, and then INCLUDE
> >> the formatting
> >>> changes. After all, there's no harm removing
> some
> >> extra spaces at
> >>> the end of lines, part of clean up.
> >>>
> >>> We often try to make things easier for the
> person
> >> who is doing a
> >>> task that we have no control over. Eg, I'd keep
> my
> >> mouth really
> >>> wide open for my dentist just so his vision is
> >> 20/20, no arguments
> >>> from me; I could afford to be slack about this
> >> "mouth wide-open
> >>> policy" if I were able to do the dentistry
> myself.
> >>>
> >>> But you're certainly right that it'll exclude
> some
> >> people,
> >>> especially folks who use editors that do not
> give
> >> very much control
> >>> to users.
> >>>
> >>> Jonathon
> >>>
> >>> Chris Howe wrote:
> >>>> I don't know that rejection policies are very
> >>>> community friendly.  Treating SVN code changes
> >> like
> >>>> the keeper of the Bridge of Death (Monty Python
> >>>> Reference, smile) may find less people willing
> to
> >> do
> >>>> in this do-ocracy.  Many of us rather like
> what's
> >> left
> >>>> of our anarco-sydicalist commune (oh look,
> >> there's
> >>>> another :-) ).
> >>>> --- Jonathon -- Improov <jo...@improov.com>
> wrote:
> >>>>> David,
> >>>>>
> >>>>> I agree with the rejection policy.
> >>>>>
> >>>>> One suggestion on procedure to take when
> >>>>> self-reviewing a patch before submitting it.
> >> Look at
> >>>>> the diff to see if there are changes we cannot
> >> account
> >>>>> for. Using KDiff also makes things easier,
> even
> >> when dealing with
> >>>>> formatting changes.
> >>>>>
> >>>>> In Emacs, it's also easy to go through every
> set
> >> of
> >>>>> changes, do an interactive-search for 1st
> entry
> >> of each set, and
> >>>>> see if the 2nd entry is
> >>>>> highlighted similarly. Meaning, if it is
> >> highlighted similarly,
> >>>>> the set of changes is bogus
> >>>>> (formatting only).
> >>>>>
> >>>>> In general, we should submit patches that are
> >>>>> intentional, ie just the intended changes
> only.
> >> We should not
> >>>>> submit patches that contain unintended
> >>>>> changes that have extra unintended footprints.
> >>>>>
> >>>>> Jonathon
> >>>>>
> >>>>> David E. Jones wrote:
> >>>>>> 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
> 
=== message truncated ===


Re: OFBiz UI work

Posted by "David E. Jones" <jo...@hotwaxmedia.com>.
Ummm... what do you think when you think of a rejection policy? We  
already have patch rejection policies when problems are found...

-David


On Jan 15, 2007, at 9:44 PM, Chris Howe wrote:

> I agree.  I think it's better that we strongly
> encourage certain practices (as we are doing now) and
> bring people to notice when those practices could be
> improved, but rejection policies put your _means
> perspective ahead of your _ends.  Contributions may be
> easier to review, but I can gaurantee you with
> rejection policies, you will then have less of it to
> review and thereby less solutions making it back into
> the project.
>
> --- "David E. Jones" <jo...@hotwaxmedia.com> wrote:
>
>>
>> Yes, we want more people, but I don't think anyone
>> wants
>> indiscriminate changes going into SVN! I hate
>> surprises when I check
>> out as much as the next guy, probably more than the
>> next guy in many
>> cases.
>>
>> Thinking about the next guy is the real key here.
>> You may want to get
>> your patches in ASAP, but if your patch breaks
>> existing code (for
>> example), then that needs to be fixed before the
>> commit is done
>> (either by the committer, the contributor, or an
>> interested third
>> party).
>>
>> So, yeah, that slows things down and is
>> inconvenient, but keep in
>> mind that a large portion of patches that come into
>> OFBiz break
>> existing functionality. This seems to happen around
>> once a week or
>> so, at least.
>>
>> It's great that people want to contribute, but in or
>> to contribute to
>> something as large and complex as OFBiz a large
>> amount of effort is
>> necessary, and anyone that wants to help out will
>> err on the side of
>> extra effort, caution and review, and consideration
>> of more general
>> requirements and designs rather just one's own
>> requirements.
>>
>> -David
>>
>>
>>
>> On Jan 15, 2007, at 7:42 PM, Jonathon -- Improov
>> wrote:
>>
>>> Heh. True. I definitely want MORE people involved
>> in OFBiz.
>>>
>>> But since I'm not a committer, I'd rather make
>> things easier for
>>> the committers. I have no control over how easy or
>> difficult it is
>>> to handle patches with "extra unintended
>> footprints".
>>>
>>> If I were a committer, I would try to
>> automatically filter out
>>> formatting changes in my audit, and then INCLUDE
>> the formatting
>>> changes. After all, there's no harm removing some
>> extra spaces at
>>> the end of lines, part of clean up.
>>>
>>> We often try to make things easier for the person
>> who is doing a
>>> task that we have no control over. Eg, I'd keep my
>> mouth really
>>> wide open for my dentist just so his vision is
>> 20/20, no arguments
>>> from me; I could afford to be slack about this
>> "mouth wide-open
>>> policy" if I were able to do the dentistry myself.
>>>
>>> But you're certainly right that it'll exclude some
>> people,
>>> especially folks who use editors that do not give
>> very much control
>>> to users.
>>>
>>> Jonathon
>>>
>>> Chris Howe wrote:
>>>> I don't know that rejection policies are very
>>>> community friendly.  Treating SVN code changes
>> like
>>>> the keeper of the Bridge of Death (Monty Python
>>>> Reference, smile) may find less people willing to
>> do
>>>> in this do-ocracy.  Many of us rather like what's
>> left
>>>> of our anarco-sydicalist commune (oh look,
>> there's
>>>> another :-) ).
>>>> --- Jonathon -- Improov <jo...@improov.com> wrote:
>>>>> David,
>>>>>
>>>>> I agree with the rejection policy.
>>>>>
>>>>> One suggestion on procedure to take when
>>>>> self-reviewing a patch before submitting it.
>> Look at
>>>>> the diff to see if there are changes we cannot
>> account
>>>>> for. Using KDiff also makes things easier, even
>> when dealing with
>>>>> formatting changes.
>>>>>
>>>>> In Emacs, it's also easy to go through every set
>> of
>>>>> changes, do an interactive-search for 1st entry
>> of each set, and
>>>>> see if the 2nd entry is
>>>>> highlighted similarly. Meaning, if it is
>> highlighted similarly,
>>>>> the set of changes is bogus
>>>>> (formatting only).
>>>>>
>>>>> In general, we should submit patches that are
>>>>> intentional, ie just the intended changes only.
>> We should not
>>>>> submit patches that contain unintended
>>>>> changes that have extra unintended footprints.
>>>>>
>>>>> Jonathon
>>>>>
>>>>> David E. Jones wrote:
>>>>>> 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
>>
> === message truncated ===
>


Re: OFBiz UI work

Posted by Chris Howe <cj...@yahoo.com>.
I agree.  I think it's better that we strongly
encourage certain practices (as we are doing now) and
bring people to notice when those practices could be
improved, but rejection policies put your _means
perspective ahead of your _ends.  Contributions may be
easier to review, but I can gaurantee you with
rejection policies, you will then have less of it to
review and thereby less solutions making it back into
the project.

--- "David E. Jones" <jo...@hotwaxmedia.com> wrote:

> 
> Yes, we want more people, but I don't think anyone
> wants  
> indiscriminate changes going into SVN! I hate
> surprises when I check  
> out as much as the next guy, probably more than the
> next guy in many  
> cases.
> 
> Thinking about the next guy is the real key here.
> You may want to get  
> your patches in ASAP, but if your patch breaks
> existing code (for  
> example), then that needs to be fixed before the
> commit is done  
> (either by the committer, the contributor, or an
> interested third  
> party).
> 
> So, yeah, that slows things down and is
> inconvenient, but keep in  
> mind that a large portion of patches that come into
> OFBiz break  
> existing functionality. This seems to happen around
> once a week or  
> so, at least.
> 
> It's great that people want to contribute, but in or
> to contribute to  
> something as large and complex as OFBiz a large
> amount of effort is  
> necessary, and anyone that wants to help out will
> err on the side of  
> extra effort, caution and review, and consideration
> of more general  
> requirements and designs rather just one's own
> requirements.
> 
> -David
> 
> 
> 
> On Jan 15, 2007, at 7:42 PM, Jonathon -- Improov
> wrote:
> 
> > Heh. True. I definitely want MORE people involved
> in OFBiz.
> >
> > But since I'm not a committer, I'd rather make
> things easier for  
> > the committers. I have no control over how easy or
> difficult it is  
> > to handle patches with "extra unintended
> footprints".
> >
> > If I were a committer, I would try to
> automatically filter out  
> > formatting changes in my audit, and then INCLUDE
> the formatting  
> > changes. After all, there's no harm removing some
> extra spaces at  
> > the end of lines, part of clean up.
> >
> > We often try to make things easier for the person
> who is doing a  
> > task that we have no control over. Eg, I'd keep my
> mouth really  
> > wide open for my dentist just so his vision is
> 20/20, no arguments  
> > from me; I could afford to be slack about this
> "mouth wide-open  
> > policy" if I were able to do the dentistry myself.
> >
> > But you're certainly right that it'll exclude some
> people,  
> > especially folks who use editors that do not give
> very much control  
> > to users.
> >
> > Jonathon
> >
> > Chris Howe wrote:
> >> I don't know that rejection policies are very
> >> community friendly.  Treating SVN code changes
> like
> >> the keeper of the Bridge of Death (Monty Python
> >> Reference, smile) may find less people willing to
> do
> >> in this do-ocracy.  Many of us rather like what's
> left
> >> of our anarco-sydicalist commune (oh look,
> there's
> >> another :-) ).
> >> --- Jonathon -- Improov <jo...@improov.com> wrote:
> >>> David,
> >>>
> >>> I agree with the rejection policy.
> >>>
> >>> One suggestion on procedure to take when
> >>> self-reviewing a patch before submitting it.
> Look at
> >>> the diff to see if there are changes we cannot
> account
> >>> for. Using KDiff also makes things easier, even
> when dealing with  
> >>> formatting changes.
> >>>
> >>> In Emacs, it's also easy to go through every set
> of
> >>> changes, do an interactive-search for 1st entry
> of each set, and  
> >>> see if the 2nd entry is
> >>> highlighted similarly. Meaning, if it is
> highlighted similarly,  
> >>> the set of changes is bogus
> >>> (formatting only).
> >>>
> >>> In general, we should submit patches that are
> >>> intentional, ie just the intended changes only.
> We should not  
> >>> submit patches that contain unintended
> >>> changes that have extra unintended footprints.
> >>>
> >>> Jonathon
> >>>
> >>> David E. Jones wrote:
> >>>> 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
> 
=== message truncated ===


Re: OFBiz UI work

Posted by "David E. Jones" <jo...@hotwaxmedia.com>.
Yes, we want more people, but I don't think anyone wants  
indiscriminate changes going into SVN! I hate surprises when I check  
out as much as the next guy, probably more than the next guy in many  
cases.

Thinking about the next guy is the real key here. You may want to get  
your patches in ASAP, but if your patch breaks existing code (for  
example), then that needs to be fixed before the commit is done  
(either by the committer, the contributor, or an interested third  
party).

So, yeah, that slows things down and is inconvenient, but keep in  
mind that a large portion of patches that come into OFBiz break  
existing functionality. This seems to happen around once a week or  
so, at least.

It's great that people want to contribute, but in or to contribute to  
something as large and complex as OFBiz a large amount of effort is  
necessary, and anyone that wants to help out will err on the side of  
extra effort, caution and review, and consideration of more general  
requirements and designs rather just one's own requirements.

-David



On Jan 15, 2007, at 7:42 PM, Jonathon -- Improov wrote:

> Heh. True. I definitely want MORE people involved in OFBiz.
>
> But since I'm not a committer, I'd rather make things easier for  
> the committers. I have no control over how easy or difficult it is  
> to handle patches with "extra unintended footprints".
>
> If I were a committer, I would try to automatically filter out  
> formatting changes in my audit, and then INCLUDE the formatting  
> changes. After all, there's no harm removing some extra spaces at  
> the end of lines, part of clean up.
>
> We often try to make things easier for the person who is doing a  
> task that we have no control over. Eg, I'd keep my mouth really  
> wide open for my dentist just so his vision is 20/20, no arguments  
> from me; I could afford to be slack about this "mouth wide-open  
> policy" if I were able to do the dentistry myself.
>
> But you're certainly right that it'll exclude some people,  
> especially folks who use editors that do not give very much control  
> to users.
>
> Jonathon
>
> Chris Howe wrote:
>> I don't know that rejection policies are very
>> community friendly.  Treating SVN code changes like
>> the keeper of the Bridge of Death (Monty Python
>> Reference, smile) may find less people willing to do
>> in this do-ocracy.  Many of us rather like what's left
>> of our anarco-sydicalist commune (oh look, there's
>> another :-) ).
>> --- Jonathon -- Improov <jo...@improov.com> wrote:
>>> David,
>>>
>>> I agree with the rejection policy.
>>>
>>> One suggestion on procedure to take when
>>> self-reviewing a patch before submitting it. Look at
>>> the diff to see if there are changes we cannot account
>>> for. Using KDiff also makes things easier, even when dealing with  
>>> formatting changes.
>>>
>>> In Emacs, it's also easy to go through every set of
>>> changes, do an interactive-search for 1st entry of each set, and  
>>> see if the 2nd entry is
>>> highlighted similarly. Meaning, if it is highlighted similarly,  
>>> the set of changes is bogus
>>> (formatting only).
>>>
>>> In general, we should submit patches that are
>>> intentional, ie just the intended changes only. We should not  
>>> submit patches that contain unintended
>>> changes that have extra unintended footprints.
>>>
>>> Jonathon
>>>
>>> David E. Jones wrote:
>>>> 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="<@o...@ofbizContentUrl>"
>>> />
>>>> +    <link rel="shortcut icon"
>> href="<@o...@ofbizContentUrl>"
>>> />
>>>>      <script language="javascript"
>> src="<@o...@ofbizContentUrl>"
>>>> type="text/javascript"></script>
>>>>      <script language="javascript"
>> src="<@o...@ofbizContentUrl>"
>>>> type="text/javascript"></script>
>>>>      <script language="javascript"
>> src="<@o...@ofbizContentUrl>"
>>>> type="text/javascript"></script>
>>>> @@ -37,8 +37,8 @@
>>>>              <link rel="stylesheet"
>> href="<@o...@ofbizContentUrl>"
>>> type="text/css"/>
>>>>          </#list>
>>>>      <#else>
>>>> +        <#-- tabstyles.css has been moved to
>>> maincss.css -->
>>>>          <link rel="stylesheet"
>> href="<@o...@ofbizContentUrl>"
>>>> type="text/css"/>
>>>> -        <link rel="stylesheet"
>> href="<@o...@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>
>>>> ======================
>>>>
>>>
>


Re: OFBiz UI work

Posted by Jonathon -- Improov <jo...@improov.com>.
Heh. True. I definitely want MORE people involved in OFBiz.

But since I'm not a committer, I'd rather make things easier for the committers. I have no control 
over how easy or difficult it is to handle patches with "extra unintended footprints".

If I were a committer, I would try to automatically filter out formatting changes in my audit, and 
then INCLUDE the formatting changes. After all, there's no harm removing some extra spaces at the 
end of lines, part of clean up.

We often try to make things easier for the person who is doing a task that we have no control 
over. Eg, I'd keep my mouth really wide open for my dentist just so his vision is 20/20, no 
arguments from me; I could afford to be slack about this "mouth wide-open policy" if I were able 
to do the dentistry myself.

But you're certainly right that it'll exclude some people, especially folks who use editors that 
do not give very much control to users.

Jonathon

Chris Howe wrote:
> I don't know that rejection policies are very
> community friendly.  Treating SVN code changes like
> the keeper of the Bridge of Death (Monty Python
> Reference, smile) may find less people willing to do
> in this do-ocracy.  Many of us rather like what's left
> of our anarco-sydicalist commune (oh look, there's
> another :-) ).
> 
> 
> --- Jonathon -- Improov <jo...@improov.com> wrote:
> 
>> David,
>>
>> I agree with the rejection policy.
>>
>> One suggestion on procedure to take when
>> self-reviewing a patch before submitting it. Look at
>> the 
>> diff to see if there are changes we cannot account
>> for. Using KDiff also makes things easier, even 
>> when dealing with formatting changes.
>>
>> In Emacs, it's also easy to go through every set of
>> changes, do an interactive-search for 1st 
>> entry of each set, and see if the 2nd entry is
>> highlighted similarly. Meaning, if it is 
>> highlighted similarly, the set of changes is bogus
>> (formatting only).
>>
>> In general, we should submit patches that are
>> intentional, ie just the intended changes only. We 
>> should not submit patches that contain unintended
>> changes that have extra unintended footprints.
>>
>> Jonathon
>>
>> David E. Jones wrote:
>>> 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="<@o...@ofbizContentUrl>"
>> />
>>> +    <link rel="shortcut icon" 
>>>
> href="<@o...@ofbizContentUrl>"
>> />
>>>      <script language="javascript" 
>>>
> src="<@o...@ofbizContentUrl>"
>>> type="text/javascript"></script>
>>>      <script language="javascript" 
>>>
> src="<@o...@ofbizContentUrl>"
>>> type="text/javascript"></script>
>>>      <script language="javascript" 
>>>
> src="<@o...@ofbizContentUrl>"
>>> type="text/javascript"></script>
>>> @@ -37,8 +37,8 @@
>>>              <link rel="stylesheet" 
>>>
> href="<@o...@ofbizContentUrl>"
>> type="text/css"/>
>>>          </#list>
>>>      <#else>
>>> +        <#-- tabstyles.css has been moved to
>> maincss.css -->
>>>          <link rel="stylesheet" 
>>>
> href="<@o...@ofbizContentUrl>"
>>> type="text/css"/>
>>> -        <link rel="stylesheet" 
>>>
> href="<@o...@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="<@o...@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>
>>> ======================
>>>
>>
> 
> 


Re: OFBiz UI work

Posted by Chris Howe <cj...@yahoo.com>.
I don't know that rejection policies are very
community friendly.  Treating SVN code changes like
the keeper of the Bridge of Death (Monty Python
Reference, smile) may find less people willing to do
in this do-ocracy.  Many of us rather like what's left
of our anarco-sydicalist commune (oh look, there's
another :-) ).


--- Jonathon -- Improov <jo...@improov.com> wrote:

> David,
> 
> I agree with the rejection policy.
> 
> One suggestion on procedure to take when
> self-reviewing a patch before submitting it. Look at
> the 
> diff to see if there are changes we cannot account
> for. Using KDiff also makes things easier, even 
> when dealing with formatting changes.
> 
> In Emacs, it's also easy to go through every set of
> changes, do an interactive-search for 1st 
> entry of each set, and see if the 2nd entry is
> highlighted similarly. Meaning, if it is 
> highlighted similarly, the set of changes is bogus
> (formatting only).
> 
> In general, we should submit patches that are
> intentional, ie just the intended changes only. We 
> should not submit patches that contain unintended
> changes that have extra unintended footprints.
> 
> Jonathon
> 
> David E. Jones wrote:
> > 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="<@o...@ofbizContentUrl>"
> />
> > +    <link rel="shortcut icon" 
> >
>
href="<@o...@ofbizContentUrl>"
> />
> >      <script language="javascript" 
> >
>
src="<@o...@ofbizContentUrl>"
> 
> > type="text/javascript"></script>
> >      <script language="javascript" 
> >
>
src="<@o...@ofbizContentUrl>"
> 
> > type="text/javascript"></script>
> >      <script language="javascript" 
> >
>
src="<@o...@ofbizContentUrl>"
> 
> > type="text/javascript"></script>
> > @@ -37,8 +37,8 @@
> >              <link rel="stylesheet" 
> >
>
href="<@o...@ofbizContentUrl>"
> type="text/css"/>
> >          </#list>
> >      <#else>
> > +        <#-- tabstyles.css has been moved to
> maincss.css -->
> >          <link rel="stylesheet" 
> >
>
href="<@o...@ofbizContentUrl>"
> 
> > type="text/css"/>
> > -        <link rel="stylesheet" 
> >
>
href="<@o...@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="<@o...@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>
> > ======================
> > 
> 
> 


Re: OFBiz UI work

Posted by Jonathon -- Improov <jo...@improov.com>.
David,

I agree with the rejection policy.

One suggestion on procedure to take when self-reviewing a patch before submitting it. Look at the 
diff to see if there are changes we cannot account for. Using KDiff also makes things easier, even 
when dealing with formatting changes.

In Emacs, it's also easy to go through every set of changes, do an interactive-search for 1st 
entry of each set, and see if the 2nd entry is highlighted similarly. Meaning, if it is 
highlighted similarly, the set of changes is bogus (formatting only).

In general, we should submit patches that are intentional, ie just the intended changes only. We 
should not submit patches that contain unintended changes that have extra unintended footprints.

Jonathon

David E. Jones wrote:
> 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="<@o...@ofbizContentUrl>" />
> +    <link rel="shortcut icon" 
> href="<@o...@ofbizContentUrl>" />
>      <script language="javascript" 
> src="<@o...@ofbizContentUrl>" 
> type="text/javascript"></script>
>      <script language="javascript" 
> src="<@o...@ofbizContentUrl>" 
> type="text/javascript"></script>
>      <script language="javascript" 
> src="<@o...@ofbizContentUrl>" 
> type="text/javascript"></script>
> @@ -37,8 +37,8 @@
>              <link rel="stylesheet" 
> href="<@o...@ofbizContentUrl>" type="text/css"/>
>          </#list>
>      <#else>
> +        <#-- tabstyles.css has been moved to maincss.css -->
>          <link rel="stylesheet" 
> href="<@o...@ofbizContentUrl>" 
> type="text/css"/>
> -        <link rel="stylesheet" 
> href="<@o...@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="<@o...@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>
> ======================
> 


Re: OFBiz UI work

Posted by Chris Howe <cj...@yahoo.com>.
Looking at his patch, I don't believe Adrian made the
formatting changes.  His editor did.  It's clearing
whitespace at the end of lines in places you wouldn't
even expect him to be touching (the license header).

Adrian, what IDE and xml editor are you using? 
There's probably a setting somewhere to prevent this
from (or causing to - depending on your
perspective)happening.


--- "David E. Jones" <jo...@hotwaxmedia.com> wrote:

> 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>
> ======================
> 
>