Re: svn commit: r925342 - in /ofbiz/trunk/applications/work
Locked
Re: svn commit: r925342 - in /ofbiz/trunk/applications/work
 
|
[hidden email] wrote:
> Author: jacopoc > Date: Fri Mar 19 17:23:15 2010 > New Revision: 925342 > > URL: http://svn.apache.org/viewvc?rev=925342&view=rev > Log: > Improved search capabilities of the work effort calendar screens; converted ftl template to form widget. This should have been 2 commits, they aren't related. > Removed: > ofbiz/trunk/applications/workeffort/webapp/workeffort/calendar/eventsByForms.ftl > Modified: > ofbiz/trunk/applications/workeffort/webapp/workeffort/WEB-INF/actions/calendar/CreateUrlParam.groovy > ofbiz/trunk/applications/workeffort/widget/CalendarForms.xml > ofbiz/trunk/applications/workeffort/widget/CommonScreens.xml > > Modified: ofbiz/trunk/applications/workeffort/webapp/workeffort/WEB-INF/actions/calendar/CreateUrlParam.groovy > URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/workeffort/webapp/workeffort/WEB-INF/actions/calendar/CreateUrlParam.groovy?rev=925342&r1=925341&r2=925342&view=diff > ============================================================================== > --- ofbiz/trunk/applications/workeffort/webapp/workeffort/WEB-INF/actions/calendar/CreateUrlParam.groovy (original) > +++ ofbiz/trunk/applications/workeffort/webapp/workeffort/WEB-INF/actions/calendar/CreateUrlParam.groovy Fri Mar 19 17:23:15 2010 > @@ -22,19 +22,28 @@ fixedAssetId = parameters.fixedAssetId; > partyId = parameters.partyId; > workEffortTypeId = parameters.workEffortTypeId; > > -urlParam = null; > +urlParam = ""; > if (facilityId) { > urlParam = "facilityId=" + facilityId; > } > if (fixedAssetId) { > - urlParam = "fixedAssetId=" + fixedAssetId; > + if (urlParam) { > + urlParam = urlParam + "&"; > + } > + urlParam = urlParam + "fixedAssetId=" + fixedAssetId; > } > if (partyId) { > - urlParam = "partyId=" + partyId; > + if (urlParam) { > + urlParam = urlParam + "&"; > + } > + urlParam = urlParam + "partyId=" + partyId; > } > > if (workEffortTypeId) { > - urlParam = "workEffortTypeId=" + workEffortTypeId; > + if (urlParam) { > + urlParam = urlParam + "&"; > + } > + urlParam = urlParam + "workEffortTypeId=" + workEffortTypeId; > } > > if (urlParam) { > > Modified: ofbiz/trunk/applications/workeffort/widget/CalendarForms.xml > URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/workeffort/widget/CalendarForms.xml?rev=925342&r1=925341&r2=925342&view=diff > ============================================================================== > --- ofbiz/trunk/applications/workeffort/widget/CalendarForms.xml (original) > +++ ofbiz/trunk/applications/workeffort/widget/CalendarForms.xml Fri Mar 19 17:23:15 2010 > @@ -20,6 +20,33 @@ under the License. > > <forms xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" > xsi:noNamespaceSchemaLocation="http://ofbiz.apache.org/dtds/widget-form.xsd"> > + <form name="FilterCalendarEvents" type="single" target="calendar"> > + <field name="partyId"> > + <lookup target-form-name="LookupPartyName" size="16"/> > + </field> > + <field name="workEffortTypeId"> > + <drop-down allow-empty="true"> > + <entity-options entity-name="WorkEffortType" description="${description}"> > + <entity-order-by field-name="description"/> > + </entity-options> > + </drop-down> > + </field> > + <field name="facilityId"> > + <drop-down allow-empty="true"> > + <entity-options entity-name="Facility" description="${facilityName}"> > + <entity-order-by field-name="facilityName"/> > + </entity-options> > + </drop-down> > + </field> > + <field name="fixedAssetId"> > + <drop-down allow-empty="true"> > + <entity-options entity-name="FixedAsset" description="${fixedAssetId} - ${fixedAssetName}"> > + <entity-order-by field-name="fixedAssetId"/> > + </entity-options> > + </drop-down> > + </field> > + <field name="viewButton" title="${uiLabelMap.CommonView}"><submit/></field> > + </form> > <!-- Forms Specific to portlet --> > <form name="EditCalendar" extends="CommonPortletEdit" extends-resource="component://common/widget/PortletEditForms.xml"> > <field name="initialView" entry-name="attributeMap.initialView"> > > Modified: ofbiz/trunk/applications/workeffort/widget/CommonScreens.xml > URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/workeffort/widget/CommonScreens.xml?rev=925342&r1=925341&r2=925342&view=diff > ============================================================================== > --- ofbiz/trunk/applications/workeffort/widget/CommonScreens.xml (original) > +++ ofbiz/trunk/applications/workeffort/widget/CommonScreens.xml Fri Mar 19 17:23:15 2010 > @@ -147,22 +147,17 @@ under the License. > <if-has-permission permission="WORKEFFORTMGR" action="_VIEW"/> > </condition> > <actions> > - <entity-condition entity-name="Facility" list="allFacilities"> > - <order-by field-name="facilityName"/> > - </entity-condition> > - <entity-condition entity-name="FixedAsset" list="allFixedAssets"> > - <order-by field-name="fixedAssetId"/> > - </entity-condition> > - <entity-condition entity-name="WorkEffortType" list="allWorkEffortTypes"> > - <order-by field-name="description"/> > - </entity-condition> > <script location="component://workeffort/webapp/workeffort/WEB-INF/actions/calendar/Days.groovy"/> > </actions> > <widgets> > - <platform-specific> > - <html><html-template location="component://workeffort/webapp/workeffort/calendar/eventsByForms.ftl"/></html> > - </platform-specific> > - <decorator-section-include name="body"/> > + <decorator-screen name="FindScreenDecorator" location="component://common/widget/CommonScreens.xml"> > + <decorator-section name="search-options"> > + <include-form name="FilterCalendarEvents" location="component://workeffort/widget/CalendarForms.xml"/> > + </decorator-section> > + <decorator-section name="search-results"> > + <decorator-section-include name="body"/> > + </decorator-section> > + </decorator-screen> > </widgets> > <fail-widgets> > <label style="h3">${uiLabelMap.WorkEffortViewPermissionError}</label> > > |
Re: svn commit: r925342 - in /ofbiz/trunk/applications/work
|
|
On Mar 19, 2010, at 6:43 PM, Adam Heath wrote: > [hidden email] wrote: >> Author: jacopoc >> Date: Fri Mar 19 17:23:15 2010 >> New Revision: 925342 >> >> URL: http://svn.apache.org/viewvc?rev=925342&view=rev >> Log: >> Improved search capabilities of the work effort calendar screens; converted ftl template to form widget. > > This should have been 2 commits, they aren't related. > I enhanced the search features by replacing the existing ftl forms (one for each search field) with *one* single widget form: in this way searches can be done with multiple constraints. Are you saying that instead I should have: 1) converted the ftl forms into widget forms in order to preserve the original limitations 2) aggregate the widget forms into one widget form in order to implement the new feature ? This is the most ridiculous thing I have ever heard. Or maybe it is a new *policy*? Jacopo >> Removed: >> ofbiz/trunk/applications/workeffort/webapp/workeffort/calendar/eventsByForms.ftl >> Modified: >> ofbiz/trunk/applications/workeffort/webapp/workeffort/WEB-INF/actions/calendar/CreateUrlParam.groovy >> ofbiz/trunk/applications/workeffort/widget/CalendarForms.xml >> ofbiz/trunk/applications/workeffort/widget/CommonScreens.xml >> >> Modified: ofbiz/trunk/applications/workeffort/webapp/workeffort/WEB-INF/actions/calendar/CreateUrlParam.groovy >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/workeffort/webapp/workeffort/WEB-INF/actions/calendar/CreateUrlParam.groovy?rev=925342&r1=925341&r2=925342&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/workeffort/webapp/workeffort/WEB-INF/actions/calendar/CreateUrlParam.groovy (original) >> +++ ofbiz/trunk/applications/workeffort/webapp/workeffort/WEB-INF/actions/calendar/CreateUrlParam.groovy Fri Mar 19 17:23:15 2010 >> @@ -22,19 +22,28 @@ fixedAssetId = parameters.fixedAssetId; >> partyId = parameters.partyId; >> workEffortTypeId = parameters.workEffortTypeId; >> >> -urlParam = null; >> +urlParam = ""; >> if (facilityId) { >> urlParam = "facilityId=" + facilityId; >> } >> if (fixedAssetId) { >> - urlParam = "fixedAssetId=" + fixedAssetId; >> + if (urlParam) { >> + urlParam = urlParam + "&"; >> + } >> + urlParam = urlParam + "fixedAssetId=" + fixedAssetId; >> } >> if (partyId) { >> - urlParam = "partyId=" + partyId; >> + if (urlParam) { >> + urlParam = urlParam + "&"; >> + } >> + urlParam = urlParam + "partyId=" + partyId; >> } >> >> if (workEffortTypeId) { >> - urlParam = "workEffortTypeId=" + workEffortTypeId; >> + if (urlParam) { >> + urlParam = urlParam + "&"; >> + } >> + urlParam = urlParam + "workEffortTypeId=" + workEffortTypeId; >> } >> >> if (urlParam) { >> >> Modified: ofbiz/trunk/applications/workeffort/widget/CalendarForms.xml >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/workeffort/widget/CalendarForms.xml?rev=925342&r1=925341&r2=925342&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/workeffort/widget/CalendarForms.xml (original) >> +++ ofbiz/trunk/applications/workeffort/widget/CalendarForms.xml Fri Mar 19 17:23:15 2010 >> @@ -20,6 +20,33 @@ under the License. >> >> <forms xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" >> xsi:noNamespaceSchemaLocation="http://ofbiz.apache.org/dtds/widget-form.xsd"> >> + <form name="FilterCalendarEvents" type="single" target="calendar"> >> + <field name="partyId"> >> + <lookup target-form-name="LookupPartyName" size="16"/> >> + </field> >> + <field name="workEffortTypeId"> >> + <drop-down allow-empty="true"> >> + <entity-options entity-name="WorkEffortType" description="${description}"> >> + <entity-order-by field-name="description"/> >> + </entity-options> >> + </drop-down> >> + </field> >> + <field name="facilityId"> >> + <drop-down allow-empty="true"> >> + <entity-options entity-name="Facility" description="${facilityName}"> >> + <entity-order-by field-name="facilityName"/> >> + </entity-options> >> + </drop-down> >> + </field> >> + <field name="fixedAssetId"> >> + <drop-down allow-empty="true"> >> + <entity-options entity-name="FixedAsset" description="${fixedAssetId} - ${fixedAssetName}"> >> + <entity-order-by field-name="fixedAssetId"/> >> + </entity-options> >> + </drop-down> >> + </field> >> + <field name="viewButton" title="${uiLabelMap.CommonView}"><submit/></field> >> + </form> >> <!-- Forms Specific to portlet --> >> <form name="EditCalendar" extends="CommonPortletEdit" extends-resource="component://common/widget/PortletEditForms.xml"> >> <field name="initialView" entry-name="attributeMap.initialView"> >> >> Modified: ofbiz/trunk/applications/workeffort/widget/CommonScreens.xml >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/workeffort/widget/CommonScreens.xml?rev=925342&r1=925341&r2=925342&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/workeffort/widget/CommonScreens.xml (original) >> +++ ofbiz/trunk/applications/workeffort/widget/CommonScreens.xml Fri Mar 19 17:23:15 2010 >> @@ -147,22 +147,17 @@ under the License. >> <if-has-permission permission="WORKEFFORTMGR" action="_VIEW"/> >> </condition> >> <actions> >> - <entity-condition entity-name="Facility" list="allFacilities"> >> - <order-by field-name="facilityName"/> >> - </entity-condition> >> - <entity-condition entity-name="FixedAsset" list="allFixedAssets"> >> - <order-by field-name="fixedAssetId"/> >> - </entity-condition> >> - <entity-condition entity-name="WorkEffortType" list="allWorkEffortTypes"> >> - <order-by field-name="description"/> >> - </entity-condition> >> <script location="component://workeffort/webapp/workeffort/WEB-INF/actions/calendar/Days.groovy"/> >> </actions> >> <widgets> >> - <platform-specific> >> - <html><html-template location="component://workeffort/webapp/workeffort/calendar/eventsByForms.ftl"/></html> >> - </platform-specific> >> - <decorator-section-include name="body"/> >> + <decorator-screen name="FindScreenDecorator" location="component://common/widget/CommonScreens.xml"> >> + <decorator-section name="search-options"> >> + <include-form name="FilterCalendarEvents" location="component://workeffort/widget/CalendarForms.xml"/> >> + </decorator-section> >> + <decorator-section name="search-results"> >> + <decorator-section-include name="body"/> >> + </decorator-section> >> + </decorator-screen> >> </widgets> >> <fail-widgets> >> <label style="h3">${uiLabelMap.WorkEffortViewPermissionError}</label> >> >> > |
Re: svn commit: r925342 - in /ofbiz/trunk/applications/work
|
|
Jacopo Cappellato wrote:
> On Mar 19, 2010, at 6:43 PM, Adam Heath wrote: > >> [hidden email] wrote: >>> Author: jacopoc >>> Date: Fri Mar 19 17:23:15 2010 >>> New Revision: 925342 >>> >>> URL: http://svn.apache.org/viewvc?rev=925342&view=rev >>> Log: >>> Improved search capabilities of the work effort calendar screens; converted ftl template to form widget. >> This should have been 2 commits, they aren't related. >> > > I enhanced the search features by replacing the existing ftl forms (one for each search field) with *one* single widget form: in this way searches can be done with multiple constraints. > Are you saying that instead I should have: > > 1) converted the ftl forms into widget forms in order to preserve the original limitations > 2) aggregate the widget forms into one widget form in order to implement the new feature > > ? > > This is the most ridiculous thing I have ever heard. Or maybe it is a new *policy*? The reason it's good to split it, is that the conversion from ftl to widget might introduce bugs, just the conversion itself. Trying to track the problem down in the future when the bug is actually found is much simpler to do when the change is isolated. All you can say about any bit of code, is that you've fixed all *known* bugs. There could be an unknown bug in the conversion, that is only discovered in the future. The split makes it simpler to track down. > > Jacopo > > > >>> Removed: >>> ofbiz/trunk/applications/workeffort/webapp/workeffort/calendar/eventsByForms.ftl >>> Modified: >>> ofbiz/trunk/applications/workeffort/webapp/workeffort/WEB-INF/actions/calendar/CreateUrlParam.groovy >>> ofbiz/trunk/applications/workeffort/widget/CalendarForms.xml >>> ofbiz/trunk/applications/workeffort/widget/CommonScreens.xml >>> >>> Modified: ofbiz/trunk/applications/workeffort/webapp/workeffort/WEB-INF/actions/calendar/CreateUrlParam.groovy >>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/workeffort/webapp/workeffort/WEB-INF/actions/calendar/CreateUrlParam.groovy?rev=925342&r1=925341&r2=925342&view=diff >>> ============================================================================== >>> --- ofbiz/trunk/applications/workeffort/webapp/workeffort/WEB-INF/actions/calendar/CreateUrlParam.groovy (original) >>> +++ ofbiz/trunk/applications/workeffort/webapp/workeffort/WEB-INF/actions/calendar/CreateUrlParam.groovy Fri Mar 19 17:23:15 2010 >>> @@ -22,19 +22,28 @@ fixedAssetId = parameters.fixedAssetId; >>> partyId = parameters.partyId; >>> workEffortTypeId = parameters.workEffortTypeId; >>> >>> -urlParam = null; >>> +urlParam = ""; >>> if (facilityId) { >>> urlParam = "facilityId=" + facilityId; >>> } >>> if (fixedAssetId) { >>> - urlParam = "fixedAssetId=" + fixedAssetId; >>> + if (urlParam) { >>> + urlParam = urlParam + "&"; >>> + } >>> + urlParam = urlParam + "fixedAssetId=" + fixedAssetId; >>> } >>> if (partyId) { >>> - urlParam = "partyId=" + partyId; >>> + if (urlParam) { >>> + urlParam = urlParam + "&"; >>> + } >>> + urlParam = urlParam + "partyId=" + partyId; >>> } >>> >>> if (workEffortTypeId) { >>> - urlParam = "workEffortTypeId=" + workEffortTypeId; >>> + if (urlParam) { >>> + urlParam = urlParam + "&"; >>> + } >>> + urlParam = urlParam + "workEffortTypeId=" + workEffortTypeId; >>> } >>> >>> if (urlParam) { >>> >>> Modified: ofbiz/trunk/applications/workeffort/widget/CalendarForms.xml >>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/workeffort/widget/CalendarForms.xml?rev=925342&r1=925341&r2=925342&view=diff >>> ============================================================================== >>> --- ofbiz/trunk/applications/workeffort/widget/CalendarForms.xml (original) >>> +++ ofbiz/trunk/applications/workeffort/widget/CalendarForms.xml Fri Mar 19 17:23:15 2010 >>> @@ -20,6 +20,33 @@ under the License. >>> >>> <forms xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" >>> xsi:noNamespaceSchemaLocation="http://ofbiz.apache.org/dtds/widget-form.xsd"> >>> + <form name="FilterCalendarEvents" type="single" target="calendar"> >>> + <field name="partyId"> >>> + <lookup target-form-name="LookupPartyName" size="16"/> >>> + </field> >>> + <field name="workEffortTypeId"> >>> + <drop-down allow-empty="true"> >>> + <entity-options entity-name="WorkEffortType" description="${description}"> >>> + <entity-order-by field-name="description"/> >>> + </entity-options> >>> + </drop-down> >>> + </field> >>> + <field name="facilityId"> >>> + <drop-down allow-empty="true"> >>> + <entity-options entity-name="Facility" description="${facilityName}"> >>> + <entity-order-by field-name="facilityName"/> >>> + </entity-options> >>> + </drop-down> >>> + </field> >>> + <field name="fixedAssetId"> >>> + <drop-down allow-empty="true"> >>> + <entity-options entity-name="FixedAsset" description="${fixedAssetId} - ${fixedAssetName}"> >>> + <entity-order-by field-name="fixedAssetId"/> >>> + </entity-options> >>> + </drop-down> >>> + </field> >>> + <field name="viewButton" title="${uiLabelMap.CommonView}"><submit/></field> >>> + </form> >>> <!-- Forms Specific to portlet --> >>> <form name="EditCalendar" extends="CommonPortletEdit" extends-resource="component://common/widget/PortletEditForms.xml"> >>> <field name="initialView" entry-name="attributeMap.initialView"> >>> >>> Modified: ofbiz/trunk/applications/workeffort/widget/CommonScreens.xml >>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/workeffort/widget/CommonScreens.xml?rev=925342&r1=925341&r2=925342&view=diff >>> ============================================================================== >>> --- ofbiz/trunk/applications/workeffort/widget/CommonScreens.xml (original) >>> +++ ofbiz/trunk/applications/workeffort/widget/CommonScreens.xml Fri Mar 19 17:23:15 2010 >>> @@ -147,22 +147,17 @@ under the License. >>> <if-has-permission permission="WORKEFFORTMGR" action="_VIEW"/> >>> </condition> >>> <actions> >>> - <entity-condition entity-name="Facility" list="allFacilities"> >>> - <order-by field-name="facilityName"/> >>> - </entity-condition> >>> - <entity-condition entity-name="FixedAsset" list="allFixedAssets"> >>> - <order-by field-name="fixedAssetId"/> >>> - </entity-condition> >>> - <entity-condition entity-name="WorkEffortType" list="allWorkEffortTypes"> >>> - <order-by field-name="description"/> >>> - </entity-condition> >>> <script location="component://workeffort/webapp/workeffort/WEB-INF/actions/calendar/Days.groovy"/> >>> </actions> >>> <widgets> >>> - <platform-specific> >>> - <html><html-template location="component://workeffort/webapp/workeffort/calendar/eventsByForms.ftl"/></html> >>> - </platform-specific> >>> - <decorator-section-include name="body"/> >>> + <decorator-screen name="FindScreenDecorator" location="component://common/widget/CommonScreens.xml"> >>> + <decorator-section name="search-options"> >>> + <include-form name="FilterCalendarEvents" location="component://workeffort/widget/CalendarForms.xml"/> >>> + </decorator-section> >>> + <decorator-section name="search-results"> >>> + <decorator-section-include name="body"/> >>> + </decorator-section> >>> + </decorator-screen> >>> </widgets> >>> <fail-widgets> >>> <label style="h3">${uiLabelMap.WorkEffortViewPermissionError}</label> >>> >>> > |
Re: svn commit: r925342 - in /ofbiz/trunk/applications/work
|
|
In reply to this post by Jacopo Cappellato-4
Jacopo Cappellato wrote:
> On Mar 19, 2010, at 6:43 PM, Adam Heath wrote: > >> [hidden email] wrote: >>> Author: jacopoc >>> Date: Fri Mar 19 17:23:15 2010 >>> New Revision: 925342 >>> >>> URL: http://svn.apache.org/viewvc?rev=925342&view=rev >>> Log: >>> Improved search capabilities of the work effort calendar screens; converted ftl template to form widget. >> This should have been 2 commits, they aren't related. >> > > I enhanced the search features by replacing the existing ftl forms (one for each search field) with *one* single widget form: in this way searches can be done with multiple constraints. > Are you saying that instead I should have: > > 1) converted the ftl forms into widget forms in order to preserve the original limitations > 2) aggregate the widget forms into one widget form in order to implement the new feature > > ? > > This is the most ridiculous thing I have ever heard. Or maybe it is a new *policy*? I also only said should, not must. Those are terms I've been familiar with because of the Debian Policy Manual; they based those terms on what RFC stuff does. |
Re: svn commit: r925342 - in /ofbiz/trunk/applications/work
|
|
In reply to this post by Adam Heath-2
On Mar 19, 2010, at 12:04 PM, Adam Heath wrote: > Jacopo Cappellato wrote: >> On Mar 19, 2010, at 6:43 PM, Adam Heath wrote: >> >>> [hidden email] wrote: >>>> Author: jacopoc >>>> Date: Fri Mar 19 17:23:15 2010 >>>> New Revision: 925342 >>>> >>>> URL: http://svn.apache.org/viewvc?rev=925342&view=rev >>>> Log: >>>> Improved search capabilities of the work effort calendar screens; converted ftl template to form widget. >>> This should have been 2 commits, they aren't related. >>> >> >> I enhanced the search features by replacing the existing ftl forms (one for each search field) with *one* single widget form: in this way searches can be done with multiple constraints. >> Are you saying that instead I should have: >> >> 1) converted the ftl forms into widget forms in order to preserve the original limitations >> 2) aggregate the widget forms into one widget form in order to implement the new feature >> >> ? >> >> This is the most ridiculous thing I have ever heard. Or maybe it is a new *policy*? > > The reason it's good to split it, is that the conversion from ftl to > widget might introduce bugs, just the conversion itself. Trying to > track the problem down in the future when the bug is actually found is > much simpler to do when the change is isolated. > > All you can say about any bit of code, is that you've fixed all > *known* bugs. There could be an unknown bug in the conversion, that > is only discovered in the future. The split makes it simpler to track > down. Adam: Maybe you need to consider a different approach to debugging, as I've mentioned before, like understanding the code. This stuff is all deterministic, there is no reason to resort to experimentation. -David |
Re: svn commit: r925342 - in /ofbiz/trunk/applications/work
|
|
In reply to this post by Adam Heath-2
On Mar 19, 2010, at 7:10 PM, Adam Heath wrote: > Jacopo Cappellato wrote: >> On Mar 19, 2010, at 6:43 PM, Adam Heath wrote: >> >>> [hidden email] wrote: >>>> Author: jacopoc >>>> Date: Fri Mar 19 17:23:15 2010 >>>> New Revision: 925342 >>>> >>>> URL: http://svn.apache.org/viewvc?rev=925342&view=rev >>>> Log: >>>> Improved search capabilities of the work effort calendar screens; converted ftl template to form widget. >>> This should have been 2 commits, they aren't related. >>> >> >> I enhanced the search features by replacing the existing ftl forms (one for each search field) with *one* single widget form: in this way searches can be done with multiple constraints. >> Are you saying that instead I should have: >> >> 1) converted the ftl forms into widget forms in order to preserve the original limitations >> 2) aggregate the widget forms into one widget form in order to implement the new feature >> >> ? >> >> This is the most ridiculous thing I have ever heard. Or maybe it is a new *policy*? > > I also only said should, not must. Those are terms I've been familiar > with because of the Debian Policy Manual; they based those terms on > what RFC stuff does. You don't need to explain to me the advantages of the basic strategy (that is *not* a policy) of doing atomic and self contained commits, whenever possible. I understand it and, as you will confirm, I use it. But in this context it doesn't apply at all. I am wasting your time at explaining this clearly evident fact to you. Adam, if you are fair you will admit that your comment to my commit was wrong, that's it. And for the future, if you will see (or you will think to see) that one of my commit doesn't respect this golden rule, since you know that I am aware of the rule, then don't waste your time warning me; it will simply mean that I have a good reason for not respecting the best practice in that commit. Jacopo |
Re: svn commit: r925342 - in /ofbiz/trunk/applications/work
|
|
In reply to this post by David E. Jones-2
David E Jones wrote:
> On Mar 19, 2010, at 12:04 PM, Adam Heath wrote: > >> Jacopo Cappellato wrote: >>> On Mar 19, 2010, at 6:43 PM, Adam Heath wrote: >>> >>>> [hidden email] wrote: >>>>> Author: jacopoc >>>>> Date: Fri Mar 19 17:23:15 2010 >>>>> New Revision: 925342 >>>>> >>>>> URL: http://svn.apache.org/viewvc?rev=925342&view=rev >>>>> Log: >>>>> Improved search capabilities of the work effort calendar screens; converted ftl template to form widget. >>>> This should have been 2 commits, they aren't related. >>>> >>> I enhanced the search features by replacing the existing ftl forms (one for each search field) with *one* single widget form: in this way searches can be done with multiple constraints. >>> Are you saying that instead I should have: >>> >>> 1) converted the ftl forms into widget forms in order to preserve the original limitations >>> 2) aggregate the widget forms into one widget form in order to implement the new feature >>> >>> ? >>> >>> This is the most ridiculous thing I have ever heard. Or maybe it is a new *policy*? >> The reason it's good to split it, is that the conversion from ftl to >> widget might introduce bugs, just the conversion itself. Trying to >> track the problem down in the future when the bug is actually found is >> much simpler to do when the change is isolated. >> >> All you can say about any bit of code, is that you've fixed all >> *known* bugs. There could be an unknown bug in the conversion, that >> is only discovered in the future. The split makes it simpler to track >> down. > > Adam: Maybe you need to consider a different approach to debugging, as I've mentioned before, like understanding the code. This stuff is all deterministic, there is no reason to resort to experimentation. (general discussion) Not going to happen, and here's why. Right now, you understand it, I understand, Jimbo over there understands it. Who cares about right now. One year from now, will you, me, Jimbo, and Billy Bob understand it? Billy Bob is a new guy that wasn't involved in this discussion, or in the change that happened, but he sees some problem that he is trying to track down. Three years from now, you're dead, I'm in an insane asylum, and Jack has a problem. Jimbo no longer works on ofbiz, and Billy Bob decided to switch to another platform. What is poor little Jack supposed to do? How can he debug it? He can't ask you, me, or Jimbo. Billy Bob could care less about ofbiz, as he decided to switch away from ofbiz because of the problem he found 2 years hence. Splitting commits into singleton, separate changes makes the job easier for people who do not have an extremely deep understanding of the code. And why is that a bad thing? One of the things I am very good at, is debugging other code, site unseen, going into whatever program, whatever code storage policies they have, and finding the actual line that is either printing the error, or causing some exception or core dump. This kind of debugging is fairly straight forward to do. Fixing the problem once the target line is identified, however, is much more difficult, and requires a deep understanding of the target code. But, if I can tell the original developers what line is the cause, what commit was the cause, it makes their job also easier. |
Re: svn commit: r925342 - in /ofbiz/trunk/applications/work
|
|
In reply to this post by Jacopo Cappellato-4
Jacopo Cappellato wrote:
> On Mar 19, 2010, at 7:10 PM, Adam Heath wrote: > >> Jacopo Cappellato wrote: >>> On Mar 19, 2010, at 6:43 PM, Adam Heath wrote: >>> >>>> [hidden email] wrote: >>>>> Author: jacopoc >>>>> Date: Fri Mar 19 17:23:15 2010 >>>>> New Revision: 925342 >>>>> >>>>> URL: http://svn.apache.org/viewvc?rev=925342&view=rev >>>>> Log: >>>>> Improved search capabilities of the work effort calendar screens; converted ftl template to form widget. >>>> This should have been 2 commits, they aren't related. >>>> >>> I enhanced the search features by replacing the existing ftl forms (one for each search field) with *one* single widget form: in this way searches can be done with multiple constraints. >>> Are you saying that instead I should have: >>> >>> 1) converted the ftl forms into widget forms in order to preserve the original limitations >>> 2) aggregate the widget forms into one widget form in order to implement the new feature >>> >>> ? >>> >>> This is the most ridiculous thing I have ever heard. Or maybe it is a new *policy*? >> I also only said should, not must. Those are terms I've been familiar >> with because of the Debian Policy Manual; they based those terms on >> what RFC stuff does. > > You don't need to explain to me the advantages of the basic strategy (that is *not* a policy) of doing atomic and self contained commits, whenever possible. > I understand it and, as you will confirm, I use it. > > But in this context it doesn't apply at all. I am wasting your time at explaining this clearly evident fact to you. > Adam, if you are fair you will admit that your comment to my commit was wrong, that's it. > > And for the future, if you will see (or you will think to see) that one of my commit doesn't respect this golden rule, since you know that I am aware of the rule, then don't waste your time warning me; > it will simply mean that I have a good reason for not respecting the best practice in that commit. Where is all this hostility coming from? I sent a simple message, saying it should be split(not must). You responded that it didn't need to be, so I assumed that you hadn't seen any of my other emails about this subject in the past(entirely possible, we are all busy, and may not read everything). So, I happily repeated myself(I have no problem doing that). You then respond with this hostile email. I see what I think are 2 separate changes in a single commit. That part was obvious from the initial email I sent. If they weren't meant to be split, then explain why. Again, it's obvious I didn't see why they could be kept together. It was evident that I didn't see it, otherwise, I wouldn't have sent that first email. I've never said that this was a golden rule. I've just explained countless times why it is better to keep things separate. Others have assumed that it has become a stick to beat people over the head with. |
Re: svn commit: r925342 - in /ofbiz/trunk/applications/work
|
|
Adam Heath wrote:
> Jacopo Cappellato wrote: >> On Mar 19, 2010, at 7:10 PM, Adam Heath wrote: >> >>> Jacopo Cappellato wrote: >>>> On Mar 19, 2010, at 6:43 PM, Adam Heath wrote: >>>> >>>>> [hidden email] wrote: >>>>>> Author: jacopoc >>>>>> Date: Fri Mar 19 17:23:15 2010 >>>>>> New Revision: 925342 >>>>>> >>>>>> URL: http://svn.apache.org/viewvc?rev=925342&view=rev >>>>>> Log: >>>>>> Improved search capabilities of the work effort calendar screens; converted ftl template to form widget. >>>>> This should have been 2 commits, they aren't related. >>>>> >>>> I enhanced the search features by replacing the existing ftl forms (one for each search field) with *one* single widget form: in this way searches can be done with multiple constraints. >>>> Are you saying that instead I should have: >>>> >>>> 1) converted the ftl forms into widget forms in order to preserve the original limitations >>>> 2) aggregate the widget forms into one widget form in order to implement the new feature >>>> >>>> ? >>>> >>>> This is the most ridiculous thing I have ever heard. Or maybe it is a new *policy*? >>> I also only said should, not must. Those are terms I've been familiar >>> with because of the Debian Policy Manual; they based those terms on >>> what RFC stuff does. >> You don't need to explain to me the advantages of the basic strategy (that is *not* a policy) of doing atomic and self contained commits, whenever possible. >> I understand it and, as you will confirm, I use it. >> >> But in this context it doesn't apply at all. I am wasting your time at explaining this clearly evident fact to you. >> Adam, if you are fair you will admit that your comment to my commit was wrong, that's it. >> >> And for the future, if you will see (or you will think to see) that one of my commit doesn't respect this golden rule, since you know that I am aware of the rule, then don't waste your time warning me; >> it will simply mean that I have a good reason for not respecting the best practice in that commit. > > Where is all this hostility coming from? I sent a simple message, > saying it should be split(not must). You responded that it didn't > need to be, so I assumed that you hadn't seen any of my other emails > about this subject in the past(entirely possible, we are all busy, and > may not read everything). So, I happily repeated myself(I have no > problem doing that). You then respond with this hostile email. > > I see what I think are 2 separate changes in a single commit. That > part was obvious from the initial email I sent. If they weren't meant > to be split, then explain why. Again, it's obvious I didn't see why > they could be kept together. It was evident that I didn't see it, > otherwise, I wouldn't have sent that first email. > > I've never said that this was a golden rule. I've just explained > countless times why it is better to keep things separate. Others have > assumed that it has become a stick to beat people over the head with. I also took time out of my schedule to read the commit message. It's good when this happens. More eyes, etc. > > |
Re: svn commit: r925342 - in /ofbiz/trunk/applications/work
|
|
On Mar 19, 2010, at 8:13 PM, Adam Heath wrote: >> >> Where is all this hostility coming from? I sent a simple message, >> saying it should be split(not must). And you were wrong. >> You responded that it didn't >> need to be, so I assumed that you hadn't seen any of my other emails >> about this subject in the past(entirely possible, we are all busy, and >> may not read everything). So, I happily repeated myself(I have no >> problem doing that). Ok, now I understand your point of view. Yes, I confirm that I also appreciate the value of singleton commits and I always try to implement them (like I did in this commit). >> You then respond with this hostile email. Yes, sorry if I have been too harsh; this happened because it took me a lot of time and energy to reply to all your emails (and Ean's ones) in the last couple of days and I was a bit disappointed (and surprised) when I had to do it again after a such simple commit. >> >> I see what I think are 2 separate changes in a single commit. That >> part was obvious from the initial email I sent. If they weren't meant >> to be split, then explain why. In fact I did it; and it took time; and you could have realized it on your own if you had spent more time reading my code. >> Again, it's obvious I didn't see why >> they could be kept together. It was evident that I didn't see it, >> otherwise, I wouldn't have sent that first email. Of course, I understand you didn't realize. >> >> I've never said that this was a golden rule. Actually I think it is a golden rule (it was not ironic) :-) But of course I am flexible and if I would see you, or Adam or Scott or David breaking it in one of their commits from time to time I would not even think to mention this; I would imply you have good reasons for doing this. Of course if I see committer X breaking this rule in each and every commit I would instead take care of raising an objection. >> I've just explained >> countless times why it is better to keep things separate. Others have >> assumed that it has become a stick to beat people over the head with. > > I also took time out of my schedule to read the commit message. It's > good when this happens. More eyes, etc. Mine too to reply to you. Jacopo > >> >> > |
Re: svn commit: r925342 - in /ofbiz/trunk/applications/work
|
|
things marked with (general) are meant to apply to anyone at all,
anytime, working on anything. Not just those in ofbiz, not just Jacopo or me. Jacopo Cappellato wrote: > On Mar 19, 2010, at 8:13 PM, Adam Heath wrote: >>> Where is all this hostility coming from? I sent a simple message, >>> saying it should be split(not must). > > And you were wrong. Ok, fine, no problem with that. But I still haven't seen a reason why it doesn't make sense to split it. Based on your own commit message, you have two separate changes. I didn't even have to look closely at what you were doing. My original email was mostly based on the message you typed in. >>> You responded that it didn't >>> need to be, so I assumed that you hadn't seen any of my other emails >>> about this subject in the past(entirely possible, we are all busy, and >>> may not read everything). So, I happily repeated myself(I have no >>> problem doing that). > > Ok, now I understand your point of view. Yes, I confirm that I also appreciate the value of singleton commits and I always try to implement them (like I did in this commit). Confirmation is nice, thanks. I believe you are the first one to actually do so. I don't require it, however. Generally, when I respond to a commit message, I don't look at who actually did it(again, repeating this). I look at *just* the single commit, all by its little lonesome self. And try to see if it can stand alone, without any other context. This is also because of the aforementioned procedure of futuring debugging, and the future person not having the full context when they start trying to figure things out. (general) >>> You then respond with this hostile email. > > Yes, sorry if I have been too harsh; this happened because it took me a lot of time and energy to reply to all your emails (and Ean's ones) in the last couple of days and I was a bit disappointed (and surprised) when I had to do it again after a such simple commit. Again, you may have considered it simple, but that is because you know different things then I do. This, as I've said, is a good thing. If someone asks a question, the very fact that they asked proves that they don't know something. If you *do* happen to know the answer, being combative/hostile/annoyed/whatever with them asking the question is a waste of time. (general) >>> I see what I think are 2 separate changes in a single commit. That >>> part was obvious from the initial email I sent. If they weren't meant >>> to be split, then explain why. > > In fact I did it; and it took time; and you could have realized it on your own if you had spent more time reading my code. I don't know what you know. We all have different overlapping knowledge bases. If I knew everything you knew, I would be you. Yes, is is theoretically possible for any one of us to understand what any one else is doing. Unfortunately, in the real world, we all have limits, so we only know certain things. They may overlap slightly, but there will always be differences in our knowledge. As the author of the change, you are the best person in the world to answer any questions that someone may have about it. If someone else says nothing, then they probably have no clue at all about it. However, if someone does respond, that means they have a partial understanding, and just need to be slightly guided to the final conclusion. (general) >>> Again, it's obvious I didn't see why >>> they could be kept together. It was evident that I didn't see it, >>> otherwise, I wouldn't have sent that first email. > > Of course, I understand you didn't realize. > >>> I've never said that this was a golden rule. > > Actually I think it is a golden rule (it was not ironic) :-) > But of course I am flexible and if I would see you, or Adam or Scott or David breaking it in one of their commits from time to time I would not even think to mention this; I would imply you have good reasons for doing this. Of course if I see committer X breaking this rule in each and every commit I would instead take care of raising an objection. Ignoring a problem is the wrong approach. Those who come up with policies/procedures/guidelines/nice-to-have may forget about them at times, and not do it correctly. This doesn't mean you shouldn't tell them about it. Go ahead and do so. They may have just made a simple, honest mistake. (general) |
Re: svn commit: r925342 - in /ofbiz/trunk/applications/work
|
|
On Mar 19, 2010, at 8:50 PM, Adam Heath wrote:
> things marked with (general) are meant to apply to anyone at all, > anytime, working on anything. Not just those in ofbiz, not just > Jacopo or me. > > Jacopo Cappellato wrote: >> On Mar 19, 2010, at 8:13 PM, Adam Heath wrote: >>>> Where is all this hostility coming from? I sent a simple message, >>>> saying it should be split(not must). >> >> And you were wrong. > > Ok, fine, no problem with that. But I still haven't seen a reason why > it doesn't make sense to split it. Based on your own commit message, > you have two separate changes. I didn't even have to look closely at > what you were doing. My original email was mostly based on the > message you typed in. Yes, I know that you just looked at the comment without looking at the code: if you had looked at the code you would have realized what I did. > >>>> You responded that it didn't >>>> need to be, so I assumed that you hadn't seen any of my other emails >>>> about this subject in the past(entirely possible, we are all busy, and >>>> may not read everything). So, I happily repeated myself(I have no >>>> problem doing that). >> >> Ok, now I understand your point of view. Yes, I confirm that I also appreciate the value of singleton commits and I always try to implement them (like I did in this commit). > > Confirmation is nice, thanks. I believe you are the first one to > actually do so. I don't require it, however. > > Generally, when I respond to a commit message, I don't look at who > actually did it(again, repeating this). I look at *just* the single > commit, all by its little lonesome self. And try to see if it can > stand alone, without any other context. This is also because of the > aforementioned procedure of futuring debugging, and the future person > not having the full context when they start trying to figure things out. > > (general) > >>>> You then respond with this hostile email. >> >> Yes, sorry if I have been too harsh; this happened because it took me a lot of time and energy to reply to all your emails (and Ean's ones) in the last couple of days and I was a bit disappointed (and surprised) when I had to do it again after a such simple commit. > > Again, you may have considered it simple, but that is because you know > different things then I do. This, as I've said, is a good thing. If > someone asks a question, the very fact that they asked proves that > they don't know something. If you *do* happen to know the answer, > being combative/hostile/annoyed/whatever with them asking the question > is a waste of time. Actually I was hostile because you didn't ask a question; your was a statement: "This should have been 2 commits, they aren't related." > > (general) > >>>> I see what I think are 2 separate changes in a single commit. That >>>> part was obvious from the initial email I sent. If they weren't meant >>>> to be split, then explain why. >> >> In fact I did it; and it took time; and you could have realized it on your own if you had spent more time reading my code. > > I don't know what you know. We all have different overlapping > knowledge bases. If I knew everything you knew, I would be you. > > Yes, is is theoretically possible for any one of us to understand what > any one else is doing. Unfortunately, in the real world, we all have > limits, so we only know certain things. They may overlap slightly, > but there will always be differences in our knowledge. > > As the author of the change, you are the best person in the world to > answer any questions that someone may have about it. If someone else > says nothing, then they probably have no clue at all about it. > However, if someone does respond, that means they have a partial > understanding, and just need to be slightly guided to the final > conclusion. > You know that I am always here to help as much as I can. > (general) > >>>> Again, it's obvious I didn't see why >>>> they could be kept together. It was evident that I didn't see it, >>>> otherwise, I wouldn't have sent that first email. >> >> Of course, I understand you didn't realize. >> >>>> I've never said that this was a golden rule. >> >> Actually I think it is a golden rule (it was not ironic) :-) >> But of course I am flexible and if I would see you, or Adam or Scott or David breaking it in one of their commits from time to time I would not even think to mention this; I would imply you have good reasons for doing this. Of course if I see committer X breaking this rule in each and every commit I would instead take care of raising an objection. > > Ignoring a problem is the wrong approach. I don't see this as a problem, just a minor defect. In fact this will cause (in the worst case scenario) a few more minutes to the hypothetical man of the future to debug the code. By the way, I think we can close the discussion now, right? I wish you a great weekend Jacopo > Those who come up with > policies/procedures/guidelines/nice-to-have may forget about them at > times, and not do it correctly. This doesn't mean you shouldn't tell > them about it. Go ahead and do so. They may have just made a simple, > honest mistake. > > (general) > |
UtilCache automated testing ... Re: svn commit: r925342 - in /ofbiz/trunk/applications/work
|
|
Jacopo Cappellato wrote:
> On Mar 19, 2010, at 8:50 PM, Adam Heath wrote: > >> things marked with (general) are meant to apply to anyone at all, >> anytime, working on anything. Not just those in ofbiz, not just >> Jacopo or me. >> >> Jacopo Cappellato wrote: >>> On Mar 19, 2010, at 8:13 PM, Adam Heath wrote: >>>>> Where is all this hostility coming from? I sent a simple message, >>>>> saying it should be split(not must). >>> And you were wrong. >> Ok, fine, no problem with that. But I still haven't seen a reason why >> it doesn't make sense to split it. Based on your own commit message, >> you have two separate changes. I didn't even have to look closely at >> what you were doing. My original email was mostly based on the >> message you typed in. > > Yes, I know that you just looked at the comment without looking at the code: if you had looked at the code you would have realized what I did. > >>>>> You responded that it didn't >>>>> need to be, so I assumed that you hadn't seen any of my other emails >>>>> about this subject in the past(entirely possible, we are all busy, and >>>>> may not read everything). So, I happily repeated myself(I have no >>>>> problem doing that). >>> Ok, now I understand your point of view. Yes, I confirm that I also appreciate the value of singleton commits and I always try to implement them (like I did in this commit). >> Confirmation is nice, thanks. I believe you are the first one to >> actually do so. I don't require it, however. >> >> Generally, when I respond to a commit message, I don't look at who >> actually did it(again, repeating this). I look at *just* the single >> commit, all by its little lonesome self. And try to see if it can >> stand alone, without any other context. This is also because of the >> aforementioned procedure of futuring debugging, and the future person >> not having the full context when they start trying to figure things out. >> >> (general) >> >>>>> You then respond with this hostile email. >>> Yes, sorry if I have been too harsh; this happened because it took me a lot of time and energy to reply to all your emails (and Ean's ones) in the last couple of days and I was a bit disappointed (and surprised) when I had to do it again after a such simple commit. >> Again, you may have considered it simple, but that is because you know >> different things then I do. This, as I've said, is a good thing. If >> someone asks a question, the very fact that they asked proves that >> they don't know something. If you *do* happen to know the answer, >> being combative/hostile/annoyed/whatever with them asking the question >> is a waste of time. > > Actually I was hostile because you didn't ask a question; your was a statement: "This should have been 2 commits, they aren't related." > >> (general) >> >>>>> I see what I think are 2 separate changes in a single commit. That >>>>> part was obvious from the initial email I sent. If they weren't meant >>>>> to be split, then explain why. >>> In fact I did it; and it took time; and you could have realized it on your own if you had spent more time reading my code. >> I don't know what you know. We all have different overlapping >> knowledge bases. If I knew everything you knew, I would be you. >> >> Yes, is is theoretically possible for any one of us to understand what >> any one else is doing. Unfortunately, in the real world, we all have >> limits, so we only know certain things. They may overlap slightly, >> but there will always be differences in our knowledge. >> >> As the author of the change, you are the best person in the world to >> answer any questions that someone may have about it. If someone else >> says nothing, then they probably have no clue at all about it. >> However, if someone does respond, that means they have a partial >> understanding, and just need to be slightly guided to the final >> conclusion. >> > > You know that I am always here to help as much as I can. > >> (general) >> >>>>> Again, it's obvious I didn't see why >>>>> they could be kept together. It was evident that I didn't see it, >>>>> otherwise, I wouldn't have sent that first email. >>> Of course, I understand you didn't realize. >>> >>>>> I've never said that this was a golden rule. >>> Actually I think it is a golden rule (it was not ironic) :-) >>> But of course I am flexible and if I would see you, or Adam or Scott or David breaking it in one of their commits from time to time I would not even think to mention this; I would imply you have good reasons for doing this. Of course if I see committer X breaking this rule in each and every commit I would instead take care of raising an objection. >> Ignoring a problem is the wrong approach. > > I don't see this as a problem, just a minor defect. In fact this will cause (in the worst case scenario) a few more minutes to the hypothetical man of the future to debug the code. > > By the way, I think we can close the discussion now, right? Sure. > > I wish you a great weekend Yeah, I get to work on UtilCache unit tests. I finally figured out how to do Soft/Weak reference testing. The docs say this: * Soft cleared before Weak. * Soft cleared before OutOfMemory. * Weak cleared after main reference goes away. So, if there is no reference to the Soft value, the only way to force it to be cleared is to actually *cause* an OutOfMemoryError to occur. So, I allocate 128k length long array(1M in size), and store each array into a List<long[]>. Then, catch the OutOfMemoryError, and do a Thread.sleep, to allow background ReferenceQueues to be notified. Weak is done the same way, by doing a test while maintaining a hard-reference, and another one with that reference eliminated. Weaks will only be cleared when the OOM happens as well(the only guaranteed way). > > Jacopo > > >> Those who come up with >> policies/procedures/guidelines/nice-to-have may forget about them at >> times, and not do it correctly. This doesn't mean you shouldn't tell >> them about it. Go ahead and do so. They may have just made a simple, >> honest mistake. >> >> (general) >> > |
«
Return to OFBiz - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |