I personally am aware of how hard it is to reviews everyone code before being accepted for a commit. So I adopted a design review from the complete team. Everyone had to review and sign off n the code before it is committed. I know that is not possible in ofbiz commit framework, however in a private company it is and takes care of all that you expressed.
Jacques Le Roux sent the following on 9/12/2011 1:59 PM: > There is an Apache way for that also: > http://www.apache.org/dev/committers.html. And a related "OFBiz way" > https://cwiki.apache.org/confluence/display/OFBADMIN/OFBiz+Contributors+Best+Practices > > > Hans is simply committing work from his team. As long as he reviews and > amend/fix the code, there is nothing we can say about it. > Some complain that code quality has lowered for some time. But this is > not only due to Hans's commits (committing contibutions is > not an easy job). My opinion on this is as long as you improve your > code, there is nothing wrong in committing lower quality code > (not bad or wrong code of course). Of course it should not break and > make wrong logic/business asumptions... Everybody has the right > to learn... Sometimes it's the hard way... Are we not here to share? > > This is my own opinion, not the PMC of course... > > Jacques > > From: "BJ Freeman" <bjf...@free-man.net> >> I have notice quite different coding styles that just does not fit one >> person committing code from Hans account. Maybe getting each an account >> would better clarify who is committing what. >> >> Jacques Le Roux sent the following on 9/12/2011 1:03 PM: >>> I don't think you should feel disqualified because you wrote the right >>> code intially! >>> Hans has an explanation, but democracy is majority. And Apache way is >>> vote and majority in such cases. Unfortunately there are no >>> other best ways than a vote. Now, as David remarked, last time we felt >>> like the United Nations in some conflicts: you need a >>> respected police or army to make the law respected... >>> >>> What I don't understand is why Hans can't keep his changes for himself >>> to let all the others happy (at least Adrian, Scott, David and I so >>> far...) >>> >>> Jacques >>> >>> From: "Adrian Crum" <adrian.c...@sandglass-software.com> >>>> Maybe a vote is in order. I am not going to initiate one because I am >>>> the author of the original code, so I feel like I am >>>> disqualified. >>>> >>>> -Adrian >>>> >>>> On 9/12/2011 8:13 PM, David E Jones wrote: >>>>> Thanks Adrian, I understand what you're getting at exactly now. >>>>> >>>>> Yes, this is frustrating isn't it, and this pattern seems to come up >>>>> over and over. That's why I like the moderated community >>>>> approach better (as opposed to the Apache way), and I guess you know >>>>> my thoughts and approach on that based on my recent efforts… >>>>> >>>>> Still, I suppose that by the Apache way we should vote on this and >>>>> consider the results binding, and make the corresponding >>>>> changes. If someone goes against that vote result, then I'm not sure >>>>> what the Apache way is… i.e. what do you do about a commit >>>>> war? >>>>> >>>>> I don't know. >>>>> >>>>> -David >>>>> >>>>> >>>>> >>>>> On Sep 12, 2011, at 12:02 PM, Adrian Crum wrote: >>>>> >>>>>> David, >>>>>> >>>>>> Keep in mind that the original design is one that you participated >>>>>> in. The agreement on the setting precedence in the original >>>>>> Jira issue was this: >>>>>> >>>>>> widget.properties -> web.xml -> URL parameters >>>>>> >>>>>> where widget.properties is the global default, which can be >>>>>> overridden by a setting in web.xml, which can be overridden by >>>>>> screen widgets or scripts or whatever (via the current context Map). >>>>>> >>>>>> The design worked great. Then Hans changed it due to a >>>>>> misunderstanding of how the design works. Despite repeated >>>>>> explanations >>>>>> of how the design works, and requests from three PMC members to >>>>>> revert his change, he refused to change it and threatened the >>>>>> community with a commit war. Since then we have had a number of >>>>>> issues reported on the mailing list describing how his change >>>>>> makes the setting unusable. >>>>>> >>>>>> It amazes me that a single -1 vote vetoes a change in the Apache >>>>>> community, but three -1 votes from PMC members can't revert >>>>>> this obvious break in software design. >>>>>> >>>>>> -Adrian >>>>>> >>>>>> On 9/12/2011 7:24 PM, Adrian Crum wrote: >>>>>>> No. The approach suggested by (and committed by) Hans is that the >>>>>>> setting in the widget.properties file overrides any other >>>>>>> setting. >>>>>>> >>>>>>> -Adrian >>>>>>> >>>>>>> On 9/12/2011 6:19 PM, David E Jones wrote: >>>>>>>> No one agrees with which approach? The approach that if you pass a >>>>>>>> widgetVerbose=true HTTP parameter that it should override >>>>>>>> the widget.properties setting? I agree with that approach… >>>>>>>> >>>>>>>> -David >>>>>>>> >>>>>>>> >>>>>>>> On Sep 12, 2011, at 6:59 AM, Adrian Crum wrote: >>>>>>>> >>>>>>>>> That's the problem - no one agrees with that approach. >>>>>>>>> >>>>>>>>> -Adrian >>>>>>>>> >>>>>>>>> On 9/12/2011 1:53 PM, Jacques Le Roux wrote: >>>>>>>>>> I think I forgot to forward Hans's answer >>>>>>>>>> >>>>>>>>>> Jacques >>>>>>>>>> >>>>>>>>>> Hans Bakker wrote: >>>>>>>>>>> On Wed, 2011-08-31 at 05:15 +0200, Jacques Le Roux wrote: >>>>>>>>>>>> widget.properties's widget.verbose setting has precedence over >>>>>>>>>>>> web.xml's widgetVerbose setting. So you can't use >>>>>>>>>>>> parameters.widgetVerbose to override widget.verbose to false. >>>>>>>>>>>> Is ModelWidget.widgetBoundaryCommentsEnabled() written this >>>>>>>>>>>> way for >>>>>>>>>>>> some reasons? >>>>>>>>>>> there was a lengthly discussion of this. As long as by >>>>>>>>>>> default the >>>>>>>>>>> properties file is not overridden in web.xml is fine either way. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> Another issue is that these HTML boundary comments get >>>>>>>>>>>> outputted even though the view handler is set to >>>>>>>>>>>> "screencsv". In >>>>>>>>>>>> the >>>>>>>>>>>> widget-screen.xsd, the only way to invoke a template to >>>>>>>>>>>> produce CSV is using<html><html-template />, but this always >>>>>>>>>>>> adds >>>>>>>>>>>> HTML >>>>>>>>>>>> comments even if the output is CSV (see HtmlWidget class). >>>>>>>>>>>> Maybe we could introduce a<csv> element or something like >>>>>>>>>>>> that? >>>>>>>>>>>> >>>>>>>>>>>> Anyway, both of those problems combined mean that there are no >>>>>>>>>>>> apparent clean ways to remove the HTML "template begin/end" >>>>>>>>>>>> boundary comments from the CSV output if you try to draw it >>>>>>>>>>>> with an *.ftl template. A workaround kludge for now is to >>>>>>>>>>>> invoke >>>>>>>>>>>> the FTL manually through a Groovy script. >>>>>>>>>>>> >>>>>>>>>>>> Thanks >>>>>>>>>>>> >>>>>>>>>>>> Jacques >>> >>> >>> > > >