Guido, From: "Guido Amarilla" <[EMAIL PROTECTED]> > 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 <[EMAIL PROTECTED]>: > > 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" <[EMAIL PROTECTED]> > > To: <dev@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 <[EMAIL PROTECTED]>: > > > > > > > > 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" <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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
