Hi All,
I finally had some time to review the functionality and code submitted under: - http://svn.apache.org/r1573884 - http://svn.apache.org/r1574400 The functionality is available in the demo environment on https://ofbiz-vm.apache.org:8443/accounting The code is related to OFBIZ-3169 <https://issues.apache.org/jira/browse/OFBIZ-3169>, but apparently the committer (Hans Bakker) didn't feel the need to follow due process by adding the improvement as a patch for review by other community members before dumping it into trunk. Though it may seem that missing budget functionality can be regarded as a bug, it should be considered an improvement and therefore it should follow the Review-then-Commit principle in stead of Commit-then-Review. Functionality overview: Basically the functionality allows users of the the accounting component to: - search and find existing budget request - create a new budget request - enhance the budget request with - budget items - budget request roles - budget request reviews - change the status of the budget request Findings: First of all, documentation describing the functionality and how to use it is not available. This is a mayor omission. Assessment of usability by end users is hindered by this lacking. Secondly, the functionality as presented works. No errors occurred when clicking on the various buttons and executing basic process steps, like: - adding removing budget items - adding and removing a review - adding and removing a role - Changing the status of the budget request But, can we say that this functionality is complete and lifts OFBiz in general and the accounting module in particular a step higher on the ladder towards best of breed in the category 'Open Source ERP Projects c.q. Solutions'? It definitely does not. The functionality appears, though included in the accounting module, to be an island. Budgets in general (and at least), are related to organisations (in OFBiz the internal organisations), P&L centers, gl accounts, and persons. This is lacking in the functionality. No internal organisation can be assigned, nor a P&L center or at the lowest a gl account. Budgets without such associations are meaningless. No permissions where incorporated in menu-items to ensure that functionalities can only be executed by appropriate users. No roles were predefined, which is customarily applicable regarding this kind of functionality. Not every users having access should be able to create budget request, or perform subsequent actions. Lacking this, the user can (must?) select any partyId and roleId to associate with the budget request. Even parties not working with (or allowed to work with) OFBiz in general and the accounting module in particular can be selected. But also due to the lacking possibility to set the internal organisation, no default currency is set. Thus, making the amounts ambiguous to interpret. Furthermore it provides only a basic workflow process consisting of (approve, reject and review). Associating a party to a budget request doesn't warrant that others can not manipulate the content/data associated with the request. When I approved a budget request, I had the button to reject available. This shouldn't be possible. And at one occasion I could click the 'reject' button multipe time to get to the final state. While status changes are registered, it should also show who invoked the status change. Due to the fact that arbitrarily any party can be associated to a budget request without any restrictions, the workflow is rendered meaningless. Conclusion(s): This solutions hasn't been thought through properly and does no good for the quality of OFBiz (both the project and the product) in general and for the accounting module in particular. Prior to dumping the code into trunk it should have been posted for review by the community, so this could have been avoided. If any other contributor to this project made such a set of functionalities available as a patch to an OFBiz issue in JIRA, it would sit there for a great length of time before it would have been committed. I advice to remove it from trunk. Regards, Pierre Smits *ORRTIZ.COM <http://www.orrtiz.com>* Services & Solutions for Cloud- Based Manufacturing, Professional Services and Retail & Trade http://www.orrtiz.com |
I don't see why it needs to be reverted. If it needs to be built out
more, then devs can supply patches for that. There have been many times in the past where some basic functionality is introduced to the project, and then it is up to the rest of the community to finish building it out. Adrian Crum Sandglass Software www.sandglass-software.com On 6/19/2014 5:26 AM, Pierre Smits wrote: > Hi All, > > I finally had some time to review the functionality and code submitted > under: > > - http://svn.apache.org/r1573884 > - http://svn.apache.org/r1574400 > > The functionality is available in the demo environment on > https://ofbiz-vm.apache.org:8443/accounting > > The code is related to OFBIZ-3169 > <https://issues.apache.org/jira/browse/OFBIZ-3169>, but apparently the > committer (Hans Bakker) didn't feel the need to follow due process by > adding the improvement as a patch for review by other community members > before dumping it into trunk. > Though it may seem that missing budget functionality can be regarded as a > bug, it should be considered an improvement and therefore it should follow > the Review-then-Commit principle in stead of Commit-then-Review. > > Functionality overview: > Basically the functionality allows users of the the accounting component to: > > - search and find existing budget request > - create a new budget request > - enhance the budget request with > - budget items > - budget request roles > - budget request reviews > - change the status of the budget request > > Findings: > First of all, documentation describing the functionality and how to use it > is not available. This is a mayor omission. Assessment of usability by end > users is hindered by this lacking. > > Secondly, the functionality as presented works. No errors occurred when > clicking on the various buttons and executing basic process steps, like: > > - adding removing budget items > - adding and removing a review > - adding and removing a role > - Changing the status of the budget request > > But, can we say that this functionality is complete and lifts OFBiz in > general and the accounting module in particular a step higher on the ladder > towards best of breed in the category 'Open Source ERP Projects c.q. > Solutions'? > > It definitely does not. The functionality appears, though included in the > accounting module, to be an island. Budgets in general (and at least), are > related to organisations (in OFBiz the internal organisations), P&L > centers, gl accounts, and persons. This is lacking in the functionality. No > internal organisation can be assigned, nor a P&L center or at the lowest a > gl account. Budgets without such associations are meaningless. > > No permissions where incorporated in menu-items to ensure that > functionalities can only be executed by appropriate users. No roles were > predefined, which is customarily applicable regarding this kind of > functionality. Not every users having access should be able to create > budget request, or perform subsequent actions. > > Lacking this, the user can (must?) select any partyId and roleId to > associate with the budget request. Even parties not working with (or > allowed to work with) OFBiz in general and the accounting module in > particular can be selected. > But also due to the lacking possibility to set the internal organisation, > no default currency is set. Thus, making the amounts ambiguous to interpret. > > Furthermore it provides only a basic workflow process consisting of > (approve, reject and review). Associating a party to a budget request > doesn't warrant that others can not manipulate the content/data associated > with the request. When I approved a budget request, I had the button to > reject available. This shouldn't be possible. And at one occasion I could > click the 'reject' button multipe time to get to the final state. > While status changes are registered, it should also show who invoked the > status change. > > Due to the fact that arbitrarily any party can be associated to a budget > request without any restrictions, the workflow is rendered meaningless. > > Conclusion(s): > This solutions hasn't been thought through properly and does no good for > the quality of OFBiz (both the project and the product) in general and for > the accounting module in particular. Prior to dumping the code into trunk > it should have been posted for review by the community, so this could have > been avoided. > > If any other contributor to this project made such a set of functionalities > available as a patch to an OFBiz issue in JIRA, it would sit there for a > great length of time before it would have been committed. > > I advice to remove it from trunk. > > Regards, > > Pierre Smits > > *ORRTIZ.COM <http://www.orrtiz.com>* > Services & Solutions for Cloud- > Based Manufacturing, Professional > Services and Retail & Trade > http://www.orrtiz.com > |
Adrian,
Thank you for your opinion. And for lowering the bar. This code dump isn't a workable solution for - as a caption on the OFBiz website states - 'the best e-commerce and Enterprise Resource Planning (ERP) software available. Instead it is a waste of time and an insult to everybody who designs and develops good, thought-through business solutions for OFBiz, and you should be offended to. Did Hans hand over his committers userId and password to one of his junior developers or the intern of the week? Because this is definitely not even close to the same level of quality and coherence as the project mgt or scrum solution. And if he didn't, it can surely not be regarded as 'going the extra mile' visavis completeness or working with the community. Pierre Smits *ORRTIZ.COM <http://www.orrtiz.com>* Services & Solutions for Cloud- Based Manufacturing, Professional Services and Retail & Trade http://www.orrtiz.com On Thu, Jun 19, 2014 at 4:19 PM, Adrian Crum < [hidden email]> wrote: > I don't see why it needs to be reverted. If it needs to be built out more, > then devs can supply patches for that. > > There have been many times in the past where some basic functionality is > introduced to the project, and then it is up to the rest of the community > to finish building it out. > > Adrian Crum > Sandglass Software > www.sandglass-software.com > > > On 6/19/2014 5:26 AM, Pierre Smits wrote: > >> Hi All, >> >> I finally had some time to review the functionality and code submitted >> under: >> >> - http://svn.apache.org/r1573884 >> - http://svn.apache.org/r1574400 >> >> >> The functionality is available in the demo environment on >> https://ofbiz-vm.apache.org:8443/accounting >> >> The code is related to OFBIZ-3169 >> <https://issues.apache.org/jira/browse/OFBIZ-3169>, but apparently the >> >> committer (Hans Bakker) didn't feel the need to follow due process by >> adding the improvement as a patch for review by other community members >> before dumping it into trunk. >> Though it may seem that missing budget functionality can be regarded as a >> bug, it should be considered an improvement and therefore it should follow >> the Review-then-Commit principle in stead of Commit-then-Review. >> >> Functionality overview: >> Basically the functionality allows users of the the accounting component >> to: >> >> - search and find existing budget request >> - create a new budget request >> - enhance the budget request with >> - budget items >> - budget request roles >> - budget request reviews >> - change the status of the budget request >> >> >> Findings: >> First of all, documentation describing the functionality and how to use it >> is not available. This is a mayor omission. Assessment of usability by end >> users is hindered by this lacking. >> >> Secondly, the functionality as presented works. No errors occurred when >> clicking on the various buttons and executing basic process steps, like: >> >> - adding removing budget items >> - adding and removing a review >> - adding and removing a role >> - Changing the status of the budget request >> >> >> But, can we say that this functionality is complete and lifts OFBiz in >> general and the accounting module in particular a step higher on the >> ladder >> towards best of breed in the category 'Open Source ERP Projects c.q. >> Solutions'? >> >> It definitely does not. The functionality appears, though included in the >> accounting module, to be an island. Budgets in general (and at least), are >> related to organisations (in OFBiz the internal organisations), P&L >> centers, gl accounts, and persons. This is lacking in the functionality. >> No >> internal organisation can be assigned, nor a P&L center or at the lowest a >> gl account. Budgets without such associations are meaningless. >> >> No permissions where incorporated in menu-items to ensure that >> functionalities can only be executed by appropriate users. No roles were >> predefined, which is customarily applicable regarding this kind of >> functionality. Not every users having access should be able to create >> budget request, or perform subsequent actions. >> >> Lacking this, the user can (must?) select any partyId and roleId to >> associate with the budget request. Even parties not working with (or >> allowed to work with) OFBiz in general and the accounting module in >> particular can be selected. >> But also due to the lacking possibility to set the internal organisation, >> no default currency is set. Thus, making the amounts ambiguous to >> interpret. >> >> Furthermore it provides only a basic workflow process consisting of >> (approve, reject and review). Associating a party to a budget request >> doesn't warrant that others can not manipulate the content/data associated >> with the request. When I approved a budget request, I had the button to >> reject available. This shouldn't be possible. And at one occasion I could >> click the 'reject' button multipe time to get to the final state. >> While status changes are registered, it should also show who invoked the >> status change. >> >> Due to the fact that arbitrarily any party can be associated to a budget >> request without any restrictions, the workflow is rendered meaningless. >> >> Conclusion(s): >> This solutions hasn't been thought through properly and does no good for >> the quality of OFBiz (both the project and the product) in general and for >> the accounting module in particular. Prior to dumping the code into trunk >> it should have been posted for review by the community, so this could have >> been avoided. >> >> If any other contributor to this project made such a set of >> functionalities >> available as a patch to an OFBiz issue in JIRA, it would sit there for a >> great length of time before it would have been committed. >> >> I advice to remove it from trunk. >> >> Regards, >> >> Pierre Smits >> >> *ORRTIZ.COM <http://www.orrtiz.com>* >> >> Services & Solutions for Cloud- >> Based Manufacturing, Professional >> Services and Retail & Trade >> http://www.orrtiz.com >> >> |
No one is lowering the bar. The problem is, you still don't understand
how an open source community works. Let me give you an example: I helped design the custom XML file format OFBiz uses for UI labels (https://issues.apache.org/jira/browse/OFBIZ-1442). There were people in the community who disagreed with the design, but it was committed anyway. For the next few months, there were a lot of commits to fix bugs that cropped up after the initial commit. Scott and David helped with the bug fixes and improvements. Eventually, the new feature was working well - but there were still hundreds of properties files that needed to be converted to the new format. That was done over a period of several years by many different people. Today, it would be hard to imagine OFBiz without the custom XML format. This is how open source works. We start with a bit of code and then everyone contributes their part to improve it. If we did things your way - code is not committed until it is perfectly complete and bug free - then the project would come to a screeching halt. You have a choice: 1. Continue trying to force everyone in the community to accept your viewpoint of how the community should work. 2. Accept how the community works and participate in it. Adrian Crum Sandglass Software www.sandglass-software.com On 6/19/2014 11:54 PM, Pierre Smits wrote: > Adrian, > > Thank you for your opinion. And for lowering the bar. > > This code dump isn't a workable solution for - as a caption on the OFBiz > website states - 'the best e-commerce and Enterprise Resource Planning > (ERP) software available. > > Instead it is a waste of time and an insult to everybody who designs and > develops good, thought-through business solutions for OFBiz, and you should > be offended to. > > Did Hans hand over his committers userId and password to one of his junior > developers or the intern of the week? Because this is definitely not even > close to the same level of quality and coherence as the project mgt or > scrum solution. And if he didn't, it can surely not be regarded as 'going > the extra mile' visavis completeness or working with the community. > > > > Pierre Smits > > *ORRTIZ.COM <http://www.orrtiz.com>* > Services & Solutions for Cloud- > Based Manufacturing, Professional > Services and Retail & Trade > http://www.orrtiz.com > > > On Thu, Jun 19, 2014 at 4:19 PM, Adrian Crum < > [hidden email]> wrote: > >> I don't see why it needs to be reverted. If it needs to be built out more, >> then devs can supply patches for that. >> >> There have been many times in the past where some basic functionality is >> introduced to the project, and then it is up to the rest of the community >> to finish building it out. >> >> Adrian Crum >> Sandglass Software >> www.sandglass-software.com >> >> >> On 6/19/2014 5:26 AM, Pierre Smits wrote: >> >>> Hi All, >>> >>> I finally had some time to review the functionality and code submitted >>> under: >>> >>> - http://svn.apache.org/r1573884 >>> - http://svn.apache.org/r1574400 >>> >>> >>> The functionality is available in the demo environment on >>> https://ofbiz-vm.apache.org:8443/accounting >>> >>> The code is related to OFBIZ-3169 >>> <https://issues.apache.org/jira/browse/OFBIZ-3169>, but apparently the >>> >>> committer (Hans Bakker) didn't feel the need to follow due process by >>> adding the improvement as a patch for review by other community members >>> before dumping it into trunk. >>> Though it may seem that missing budget functionality can be regarded as a >>> bug, it should be considered an improvement and therefore it should follow >>> the Review-then-Commit principle in stead of Commit-then-Review. >>> >>> Functionality overview: >>> Basically the functionality allows users of the the accounting component >>> to: >>> >>> - search and find existing budget request >>> - create a new budget request >>> - enhance the budget request with >>> - budget items >>> - budget request roles >>> - budget request reviews >>> - change the status of the budget request >>> >>> >>> Findings: >>> First of all, documentation describing the functionality and how to use it >>> is not available. This is a mayor omission. Assessment of usability by end >>> users is hindered by this lacking. >>> >>> Secondly, the functionality as presented works. No errors occurred when >>> clicking on the various buttons and executing basic process steps, like: >>> >>> - adding removing budget items >>> - adding and removing a review >>> - adding and removing a role >>> - Changing the status of the budget request >>> >>> >>> But, can we say that this functionality is complete and lifts OFBiz in >>> general and the accounting module in particular a step higher on the >>> ladder >>> towards best of breed in the category 'Open Source ERP Projects c.q. >>> Solutions'? >>> >>> It definitely does not. The functionality appears, though included in the >>> accounting module, to be an island. Budgets in general (and at least), are >>> related to organisations (in OFBiz the internal organisations), P&L >>> centers, gl accounts, and persons. This is lacking in the functionality. >>> No >>> internal organisation can be assigned, nor a P&L center or at the lowest a >>> gl account. Budgets without such associations are meaningless. >>> >>> No permissions where incorporated in menu-items to ensure that >>> functionalities can only be executed by appropriate users. No roles were >>> predefined, which is customarily applicable regarding this kind of >>> functionality. Not every users having access should be able to create >>> budget request, or perform subsequent actions. >>> >>> Lacking this, the user can (must?) select any partyId and roleId to >>> associate with the budget request. Even parties not working with (or >>> allowed to work with) OFBiz in general and the accounting module in >>> particular can be selected. >>> But also due to the lacking possibility to set the internal organisation, >>> no default currency is set. Thus, making the amounts ambiguous to >>> interpret. >>> >>> Furthermore it provides only a basic workflow process consisting of >>> (approve, reject and review). Associating a party to a budget request >>> doesn't warrant that others can not manipulate the content/data associated >>> with the request. When I approved a budget request, I had the button to >>> reject available. This shouldn't be possible. And at one occasion I could >>> click the 'reject' button multipe time to get to the final state. >>> While status changes are registered, it should also show who invoked the >>> status change. >>> >>> Due to the fact that arbitrarily any party can be associated to a budget >>> request without any restrictions, the workflow is rendered meaningless. >>> >>> Conclusion(s): >>> This solutions hasn't been thought through properly and does no good for >>> the quality of OFBiz (both the project and the product) in general and for >>> the accounting module in particular. Prior to dumping the code into trunk >>> it should have been posted for review by the community, so this could have >>> been avoided. >>> >>> If any other contributor to this project made such a set of >>> functionalities >>> available as a patch to an OFBiz issue in JIRA, it would sit there for a >>> great length of time before it would have been committed. >>> >>> I advice to remove it from trunk. >>> >>> Regards, >>> >>> Pierre Smits >>> >>> *ORRTIZ.COM <http://www.orrtiz.com>* >>> >>> Services & Solutions for Cloud- >>> Based Manufacturing, Professional >>> Services and Retail & Trade >>> http://www.orrtiz.com >>> >>> > |
On 06/20/2014 10:42 AM, Adrian Crum wrote:
> No one is lowering the bar. The problem is, you still don't understand > how an open source community works. This is wrong. There is no hard-set one-workflow-to-bind-them methodology here. Some people seem to think there is. The Review-Commit workflow people sometimes want all stuff reviewed. While that can help find many issues ahead of time, it will never be able to find and implement all corner cases. That can generally only happen when the isolated code is pushed into the wider community. At some point, you just need to push your reviewed code to someone else, and continue to fix/improve afterwards. The Commit-Review workflow can sometimes cause severe breakage for other users. If not-ready code is pushed into trunk, and it causes feature breakage, or compilation problems, or corruption, the community at large might not be able to help, as the newly pushed code is unknown, and it'll take a while to come up to speed. Speaking about one earlier point: Committing code *early* does not lower the bar for the committed code. Code is code. It is, or it isn't. When something is committed has no bearing on the quality of the commit. That quality is a separate item from the time of the commit. > Let me give you an example: > > I helped design the custom XML file format OFBiz uses for UI labels > (https://issues.apache.org/jira/browse/OFBIZ-1442). There were people > in the community who disagreed with the design, but it was committed > anyway. Even here, you didn't do this initial work *in trunk*. You thought about the idea for a while, tried some things, got an initial set, then eventually committed to trunk. > For the next few months, there were a lot of commits to fix bugs that > cropped up after the initial commit. Scott and David helped with the > bug fixes and improvements. > > Eventually, the new feature was working well - but there were still > hundreds of properties files that needed to be converted to the new > format. That was done over a period of several years by many different > people. Another concrete example. Currently, I'm working on fixes to the entityengine crypt system. 2 years ago, I implemented key-encrypting-key support(read up on the credit card number security docs). I worked on the code in isolation, on behalf of a customer. Once it worked there, I then directly pushed to ofbiz trunk. This is the Commit-Review workflow. No review happened. None. If such review had happened, it might have discovered that direct lookup of encrypted fields would have been broken by my new code(a random salt is prepended to each encrypted value, which prevents lookups). Even if that review *had* happened, after the commit, or even *before* the commit, and I didn't add that random salt, it *still* wouldn't have fixed the direct lookup problem. This was due to direct-lookup being broken as far back as release4.0, and probably even all the way back to 2005(!). This points to a general lack of review, at *all* levels. It's been my experience, that the Review-Commit workflow will get you some small about of early reviews; those people who are keenly interested in your new idea will possibly wake up and comment on it. However, the Commit-Review can get you *more* reviewers; in addition to those who are interested in the new stuff, you'll find completely random people might speak up, and offer some un-thought-of problem. Even simple things, like a null pointer check happening after a dereference can be found this way. |
To clarify: I wasn't trying to push for a particular methodology. I was
trying to address Pierre's approach to this community, where he demands that any code he doesn't like be reverted (he has done this before). Traditionally, that is not how we do things in this community. I agree that a commit that breaks the build process or severely impacts the operation of the application should be fixed as quickly as possible or it should be reverted. That is not the case in this thread. The new functionality might not meet Pierre's standards, but it isn't breaking the build. Therefore, I don't agree that it should be reverted. Adrian Crum Sandglass Software www.sandglass-software.com On 6/20/2014 9:01 AM, Adam Heath wrote: > On 06/20/2014 10:42 AM, Adrian Crum wrote: >> No one is lowering the bar. The problem is, you still don't understand >> how an open source community works. > > This is wrong. There is no hard-set one-workflow-to-bind-them > methodology here. Some people seem to think there is. > > The Review-Commit workflow people sometimes want all stuff reviewed. > While that can help find many issues ahead of time, it will never be > able to find and implement all corner cases. That can generally only > happen when the isolated code is pushed into the wider community. At > some point, you just need to push your reviewed code to someone else, > and continue to fix/improve afterwards. > > The Commit-Review workflow can sometimes cause severe breakage for other > users. If not-ready code is pushed into trunk, and it causes feature > breakage, or compilation problems, or corruption, the community at large > might not be able to help, as the newly pushed code is unknown, and > it'll take a while to come up to speed. > > Speaking about one earlier point: Committing code *early* does not lower > the bar for the committed code. Code is code. It is, or it isn't. When > something is committed has no bearing on the quality of the commit. > That quality is a separate item from the time of the commit. > >> Let me give you an example: >> >> I helped design the custom XML file format OFBiz uses for UI labels >> (https://issues.apache.org/jira/browse/OFBIZ-1442). There were people >> in the community who disagreed with the design, but it was committed >> anyway. > > Even here, you didn't do this initial work *in trunk*. You thought > about the idea for a while, tried some things, got an initial set, then > eventually committed to trunk. > >> For the next few months, there were a lot of commits to fix bugs that >> cropped up after the initial commit. Scott and David helped with the >> bug fixes and improvements. >> >> Eventually, the new feature was working well - but there were still >> hundreds of properties files that needed to be converted to the new >> format. That was done over a period of several years by many different >> people. > > Another concrete example. > > Currently, I'm working on fixes to the entityengine crypt system. > > 2 years ago, I implemented key-encrypting-key support(read up on the > credit card number security docs). I worked on the code in isolation, > on behalf of a customer. Once it worked there, I then directly pushed > to ofbiz trunk. This is the Commit-Review workflow. > > No review happened. None. If such review had happened, it might have > discovered that direct lookup of encrypted fields would have been broken > by my new code(a random salt is prepended to each encrypted value, which > prevents lookups). > > Even if that review *had* happened, after the commit, or even *before* > the commit, and I didn't add that random salt, it *still* wouldn't have > fixed the direct lookup problem. This was due to direct-lookup being > broken as far back as release4.0, and probably even all the way back to > 2005(!). This points to a general lack of review, at *all* levels. > > It's been my experience, that the Review-Commit workflow will get you > some small about of early reviews; those people who are keenly > interested in your new idea will possibly wake up and comment on it. > However, the Commit-Review can get you *more* reviewers; in addition to > those who are interested in the new stuff, you'll find completely random > people might speak up, and offer some un-thought-of problem. Even > simple things, like a null pointer check happening after a dereference > can be found this way. |
Administrator
|
On the other hand degrading the software quality of OFBiz is not good as well.
For instance, did someone ever get the "new" Product "Images Management" to work? how to upload, etc. ? When someone new to OFBiz, like Ron recently, look at this feature, try to use it, see it does work not as expected and can't find any information about it, s/he can reasonably doubt about the whole quality of OFBiz. Of course you, I and others could fill the missing pieces. But we are not even sure this has even worked one day (It has surely but obviously it's no longer OOTB). Recently we decided about slimming down OFBiz. I see no reasons to allow committers to dump botched work in OFBiz if this means another slimming down session in few years :( Even more it these committers don't care at all after having dumped their crap! Thank you :/ Jacques Le 20/06/2014 18:32, Adrian Crum a écrit : > To clarify: I wasn't trying to push for a particular methodology. I was trying to address Pierre's approach to this community, where he demands that > any code he doesn't like be reverted (he has done this before). Traditionally, that is not how we do things in this community. > > I agree that a commit that breaks the build process or severely impacts the operation of the application should be fixed as quickly as possible or > it should be reverted. That is not the case in this thread. The new functionality might not meet Pierre's standards, but it isn't breaking the > build. Therefore, I don't agree that it should be reverted. > > Adrian Crum > Sandglass Software > www.sandglass-software.com > > On 6/20/2014 9:01 AM, Adam Heath wrote: >> On 06/20/2014 10:42 AM, Adrian Crum wrote: >>> No one is lowering the bar. The problem is, you still don't understand >>> how an open source community works. >> >> This is wrong. There is no hard-set one-workflow-to-bind-them >> methodology here. Some people seem to think there is. >> >> The Review-Commit workflow people sometimes want all stuff reviewed. >> While that can help find many issues ahead of time, it will never be >> able to find and implement all corner cases. That can generally only >> happen when the isolated code is pushed into the wider community. At >> some point, you just need to push your reviewed code to someone else, >> and continue to fix/improve afterwards. >> >> The Commit-Review workflow can sometimes cause severe breakage for other >> users. If not-ready code is pushed into trunk, and it causes feature >> breakage, or compilation problems, or corruption, the community at large >> might not be able to help, as the newly pushed code is unknown, and >> it'll take a while to come up to speed. >> >> Speaking about one earlier point: Committing code *early* does not lower >> the bar for the committed code. Code is code. It is, or it isn't. When >> something is committed has no bearing on the quality of the commit. >> That quality is a separate item from the time of the commit. >> >>> Let me give you an example: >>> >>> I helped design the custom XML file format OFBiz uses for UI labels >>> (https://issues.apache.org/jira/browse/OFBIZ-1442). There were people >>> in the community who disagreed with the design, but it was committed >>> anyway. >> >> Even here, you didn't do this initial work *in trunk*. You thought >> about the idea for a while, tried some things, got an initial set, then >> eventually committed to trunk. >> >>> For the next few months, there were a lot of commits to fix bugs that >>> cropped up after the initial commit. Scott and David helped with the >>> bug fixes and improvements. >>> >>> Eventually, the new feature was working well - but there were still >>> hundreds of properties files that needed to be converted to the new >>> format. That was done over a period of several years by many different >>> people. >> >> Another concrete example. >> >> Currently, I'm working on fixes to the entityengine crypt system. >> >> 2 years ago, I implemented key-encrypting-key support(read up on the >> credit card number security docs). I worked on the code in isolation, >> on behalf of a customer. Once it worked there, I then directly pushed >> to ofbiz trunk. This is the Commit-Review workflow. >> >> No review happened. None. If such review had happened, it might have >> discovered that direct lookup of encrypted fields would have been broken >> by my new code(a random salt is prepended to each encrypted value, which >> prevents lookups). >> >> Even if that review *had* happened, after the commit, or even *before* >> the commit, and I didn't add that random salt, it *still* wouldn't have >> fixed the direct lookup problem. This was due to direct-lookup being >> broken as far back as release4.0, and probably even all the way back to >> 2005(!). This points to a general lack of review, at *all* levels. >> >> It's been my experience, that the Review-Commit workflow will get you >> some small about of early reviews; those people who are keenly >> interested in your new idea will possibly wake up and comment on it. >> However, the Commit-Review can get you *more* reviewers; in addition to >> those who are interested in the new stuff, you'll find completely random >> people might speak up, and offer some un-thought-of problem. Even >> simple things, like a null pointer check happening after a dereference >> can be found this way. > -- |
In reply to this post by Adrian Crum-3
To clarify: I didn't demand that this code was removed. I advised to
removed it. Pierre Smits *ORRTIZ.COM <http://www.orrtiz.com>* Services & Solutions for Cloud- Based Manufacturing, Professional Services and Retail & Trade http://www.orrtiz.com On Fri, Jun 20, 2014 at 6:32 PM, Adrian Crum < [hidden email]> wrote: > To clarify: I wasn't trying to push for a particular methodology. I was > trying to address Pierre's approach to this community, where he demands > that any code he doesn't like be reverted (he has done this before). > Traditionally, that is not how we do things in this community. > > I agree that a commit that breaks the build process or severely impacts > the operation of the application should be fixed as quickly as possible or > it should be reverted. That is not the case in this thread. The new > functionality might not meet Pierre's standards, but it isn't breaking the > build. Therefore, I don't agree that it should be reverted. > > > Adrian Crum > Sandglass Software > www.sandglass-software.com > > On 6/20/2014 9:01 AM, Adam Heath wrote: > >> On 06/20/2014 10:42 AM, Adrian Crum wrote: >> >>> No one is lowering the bar. The problem is, you still don't understand >>> how an open source community works. >>> >> >> This is wrong. There is no hard-set one-workflow-to-bind-them >> methodology here. Some people seem to think there is. >> >> The Review-Commit workflow people sometimes want all stuff reviewed. >> While that can help find many issues ahead of time, it will never be >> able to find and implement all corner cases. That can generally only >> happen when the isolated code is pushed into the wider community. At >> some point, you just need to push your reviewed code to someone else, >> and continue to fix/improve afterwards. >> >> The Commit-Review workflow can sometimes cause severe breakage for other >> users. If not-ready code is pushed into trunk, and it causes feature >> breakage, or compilation problems, or corruption, the community at large >> might not be able to help, as the newly pushed code is unknown, and >> it'll take a while to come up to speed. >> >> Speaking about one earlier point: Committing code *early* does not lower >> the bar for the committed code. Code is code. It is, or it isn't. When >> something is committed has no bearing on the quality of the commit. >> That quality is a separate item from the time of the commit. >> >> Let me give you an example: >>> >>> I helped design the custom XML file format OFBiz uses for UI labels >>> (https://issues.apache.org/jira/browse/OFBIZ-1442). There were people >>> in the community who disagreed with the design, but it was committed >>> anyway. >>> >> >> Even here, you didn't do this initial work *in trunk*. You thought >> about the idea for a while, tried some things, got an initial set, then >> eventually committed to trunk. >> >> For the next few months, there were a lot of commits to fix bugs that >>> cropped up after the initial commit. Scott and David helped with the >>> bug fixes and improvements. >>> >>> Eventually, the new feature was working well - but there were still >>> hundreds of properties files that needed to be converted to the new >>> format. That was done over a period of several years by many different >>> people. >>> >> >> Another concrete example. >> >> Currently, I'm working on fixes to the entityengine crypt system. >> >> 2 years ago, I implemented key-encrypting-key support(read up on the >> credit card number security docs). I worked on the code in isolation, >> on behalf of a customer. Once it worked there, I then directly pushed >> to ofbiz trunk. This is the Commit-Review workflow. >> >> No review happened. None. If such review had happened, it might have >> discovered that direct lookup of encrypted fields would have been broken >> by my new code(a random salt is prepended to each encrypted value, which >> prevents lookups). >> >> Even if that review *had* happened, after the commit, or even *before* >> the commit, and I didn't add that random salt, it *still* wouldn't have >> fixed the direct lookup problem. This was due to direct-lookup being >> broken as far back as release4.0, and probably even all the way back to >> 2005(!). This points to a general lack of review, at *all* levels. >> >> It's been my experience, that the Review-Commit workflow will get you >> some small about of early reviews; those people who are keenly >> interested in your new idea will possibly wake up and comment on it. >> However, the Commit-Review can get you *more* reviewers; in addition to >> those who are interested in the new stuff, you'll find completely random >> people might speak up, and offer some un-thought-of problem. Even >> simple things, like a null pointer check happening after a dereference >> can be found this way. >> > |
In reply to this post by Pierre Smits
The Budget data model has been in place for over 8 years, someone finally makes an attempt to build it out a little with some basic screens, services and data and you want that reverted?
This is a step in the right direction, nothing more. If you expect every improvement to be some sort of a step across the feature finish line then you're going to be disappointed over and over again because the community has never worked that way. The only reason for a revert is when the step is in the wrong direction and will make working towards the finish line more difficult for future contributions. I cannot fathom how that would be the case here given that the implementation are basic and virtually no design designs have been made (aside from a few StatusItem records that can easily be changed/added to). Regards Scott On 20/06/2014, at 9:54 am, Pierre Smits <[hidden email]> wrote: > Adrian, > > Thank you for your opinion. And for lowering the bar. > > This code dump isn't a workable solution for - as a caption on the OFBiz > website states - 'the best e-commerce and Enterprise Resource Planning > (ERP) software available. > > Instead it is a waste of time and an insult to everybody who designs and > develops good, thought-through business solutions for OFBiz, and you should > be offended to. > > Did Hans hand over his committers userId and password to one of his junior > developers or the intern of the week? Because this is definitely not even > close to the same level of quality and coherence as the project mgt or > scrum solution. And if he didn't, it can surely not be regarded as 'going > the extra mile' visavis completeness or working with the community. > > > > Pierre Smits > > *ORRTIZ.COM <http://www.orrtiz.com>* > Services & Solutions for Cloud- > Based Manufacturing, Professional > Services and Retail & Trade > http://www.orrtiz.com > > > On Thu, Jun 19, 2014 at 4:19 PM, Adrian Crum < > [hidden email]> wrote: > >> I don't see why it needs to be reverted. If it needs to be built out more, >> then devs can supply patches for that. >> >> There have been many times in the past where some basic functionality is >> introduced to the project, and then it is up to the rest of the community >> to finish building it out. >> >> Adrian Crum >> Sandglass Software >> www.sandglass-software.com >> >> >> On 6/19/2014 5:26 AM, Pierre Smits wrote: >> >>> Hi All, >>> >>> I finally had some time to review the functionality and code submitted >>> under: >>> >>> - http://svn.apache.org/r1573884 >>> - http://svn.apache.org/r1574400 >>> >>> >>> The functionality is available in the demo environment on >>> https://ofbiz-vm.apache.org:8443/accounting >>> >>> The code is related to OFBIZ-3169 >>> <https://issues.apache.org/jira/browse/OFBIZ-3169>, but apparently the >>> >>> committer (Hans Bakker) didn't feel the need to follow due process by >>> adding the improvement as a patch for review by other community members >>> before dumping it into trunk. >>> Though it may seem that missing budget functionality can be regarded as a >>> bug, it should be considered an improvement and therefore it should follow >>> the Review-then-Commit principle in stead of Commit-then-Review. >>> >>> Functionality overview: >>> Basically the functionality allows users of the the accounting component >>> to: >>> >>> - search and find existing budget request >>> - create a new budget request >>> - enhance the budget request with >>> - budget items >>> - budget request roles >>> - budget request reviews >>> - change the status of the budget request >>> >>> >>> Findings: >>> First of all, documentation describing the functionality and how to use it >>> is not available. This is a mayor omission. Assessment of usability by end >>> users is hindered by this lacking. >>> >>> Secondly, the functionality as presented works. No errors occurred when >>> clicking on the various buttons and executing basic process steps, like: >>> >>> - adding removing budget items >>> - adding and removing a review >>> - adding and removing a role >>> - Changing the status of the budget request >>> >>> >>> But, can we say that this functionality is complete and lifts OFBiz in >>> general and the accounting module in particular a step higher on the >>> ladder >>> towards best of breed in the category 'Open Source ERP Projects c.q. >>> Solutions'? >>> >>> It definitely does not. The functionality appears, though included in the >>> accounting module, to be an island. Budgets in general (and at least), are >>> related to organisations (in OFBiz the internal organisations), P&L >>> centers, gl accounts, and persons. This is lacking in the functionality. >>> No >>> internal organisation can be assigned, nor a P&L center or at the lowest a >>> gl account. Budgets without such associations are meaningless. >>> >>> No permissions where incorporated in menu-items to ensure that >>> functionalities can only be executed by appropriate users. No roles were >>> predefined, which is customarily applicable regarding this kind of >>> functionality. Not every users having access should be able to create >>> budget request, or perform subsequent actions. >>> >>> Lacking this, the user can (must?) select any partyId and roleId to >>> associate with the budget request. Even parties not working with (or >>> allowed to work with) OFBiz in general and the accounting module in >>> particular can be selected. >>> But also due to the lacking possibility to set the internal organisation, >>> no default currency is set. Thus, making the amounts ambiguous to >>> interpret. >>> >>> Furthermore it provides only a basic workflow process consisting of >>> (approve, reject and review). Associating a party to a budget request >>> doesn't warrant that others can not manipulate the content/data associated >>> with the request. When I approved a budget request, I had the button to >>> reject available. This shouldn't be possible. And at one occasion I could >>> click the 'reject' button multipe time to get to the final state. >>> While status changes are registered, it should also show who invoked the >>> status change. >>> >>> Due to the fact that arbitrarily any party can be associated to a budget >>> request without any restrictions, the workflow is rendered meaningless. >>> >>> Conclusion(s): >>> This solutions hasn't been thought through properly and does no good for >>> the quality of OFBiz (both the project and the product) in general and for >>> the accounting module in particular. Prior to dumping the code into trunk >>> it should have been posted for review by the community, so this could have >>> been avoided. >>> >>> If any other contributor to this project made such a set of >>> functionalities >>> available as a patch to an OFBiz issue in JIRA, it would sit there for a >>> great length of time before it would have been committed. >>> >>> I advice to remove it from trunk. >>> >>> Regards, >>> >>> Pierre Smits >>> >>> *ORRTIZ.COM <http://www.orrtiz.com>* >>> >>> Services & Solutions for Cloud- >>> Based Manufacturing, Professional >>> Services and Retail & Trade >>> http://www.orrtiz.com >>> >>> |
Typo: "design designs" -> "design decisions"
Regards Scott On 10/08/2014, at 11:25 am, Scott Gray <[hidden email]> wrote: > The Budget data model has been in place for over 8 years, someone finally makes an attempt to build it out a little with some basic screens, services and data and you want that reverted? > > This is a step in the right direction, nothing more. If you expect every improvement to be some sort of a step across the feature finish line then you're going to be disappointed over and over again because the community has never worked that way. > > The only reason for a revert is when the step is in the wrong direction and will make working towards the finish line more difficult for future contributions. I cannot fathom how that would be the case here given that the implementation are basic and virtually no design designs have been made (aside from a few StatusItem records that can easily be changed/added to). > > Regards > Scott > > On 20/06/2014, at 9:54 am, Pierre Smits <[hidden email]> wrote: > >> Adrian, >> >> Thank you for your opinion. And for lowering the bar. >> >> This code dump isn't a workable solution for - as a caption on the OFBiz >> website states - 'the best e-commerce and Enterprise Resource Planning >> (ERP) software available. >> >> Instead it is a waste of time and an insult to everybody who designs and >> develops good, thought-through business solutions for OFBiz, and you should >> be offended to. >> >> Did Hans hand over his committers userId and password to one of his junior >> developers or the intern of the week? Because this is definitely not even >> close to the same level of quality and coherence as the project mgt or >> scrum solution. And if he didn't, it can surely not be regarded as 'going >> the extra mile' visavis completeness or working with the community. >> >> >> >> Pierre Smits >> >> *ORRTIZ.COM <http://www.orrtiz.com>* >> Services & Solutions for Cloud- >> Based Manufacturing, Professional >> Services and Retail & Trade >> http://www.orrtiz.com >> >> >> On Thu, Jun 19, 2014 at 4:19 PM, Adrian Crum < >> [hidden email]> wrote: >> >>> I don't see why it needs to be reverted. If it needs to be built out more, >>> then devs can supply patches for that. >>> >>> There have been many times in the past where some basic functionality is >>> introduced to the project, and then it is up to the rest of the community >>> to finish building it out. >>> >>> Adrian Crum >>> Sandglass Software >>> www.sandglass-software.com >>> >>> >>> On 6/19/2014 5:26 AM, Pierre Smits wrote: >>> >>>> Hi All, >>>> >>>> I finally had some time to review the functionality and code submitted >>>> under: >>>> >>>> - http://svn.apache.org/r1573884 >>>> - http://svn.apache.org/r1574400 >>>> >>>> >>>> The functionality is available in the demo environment on >>>> https://ofbiz-vm.apache.org:8443/accounting >>>> >>>> The code is related to OFBIZ-3169 >>>> <https://issues.apache.org/jira/browse/OFBIZ-3169>, but apparently the >>>> >>>> committer (Hans Bakker) didn't feel the need to follow due process by >>>> adding the improvement as a patch for review by other community members >>>> before dumping it into trunk. >>>> Though it may seem that missing budget functionality can be regarded as a >>>> bug, it should be considered an improvement and therefore it should follow >>>> the Review-then-Commit principle in stead of Commit-then-Review. >>>> >>>> Functionality overview: >>>> Basically the functionality allows users of the the accounting component >>>> to: >>>> >>>> - search and find existing budget request >>>> - create a new budget request >>>> - enhance the budget request with >>>> - budget items >>>> - budget request roles >>>> - budget request reviews >>>> - change the status of the budget request >>>> >>>> >>>> Findings: >>>> First of all, documentation describing the functionality and how to use it >>>> is not available. This is a mayor omission. Assessment of usability by end >>>> users is hindered by this lacking. >>>> >>>> Secondly, the functionality as presented works. No errors occurred when >>>> clicking on the various buttons and executing basic process steps, like: >>>> >>>> - adding removing budget items >>>> - adding and removing a review >>>> - adding and removing a role >>>> - Changing the status of the budget request >>>> >>>> >>>> But, can we say that this functionality is complete and lifts OFBiz in >>>> general and the accounting module in particular a step higher on the >>>> ladder >>>> towards best of breed in the category 'Open Source ERP Projects c.q. >>>> Solutions'? >>>> >>>> It definitely does not. The functionality appears, though included in the >>>> accounting module, to be an island. Budgets in general (and at least), are >>>> related to organisations (in OFBiz the internal organisations), P&L >>>> centers, gl accounts, and persons. This is lacking in the functionality. >>>> No >>>> internal organisation can be assigned, nor a P&L center or at the lowest a >>>> gl account. Budgets without such associations are meaningless. >>>> >>>> No permissions where incorporated in menu-items to ensure that >>>> functionalities can only be executed by appropriate users. No roles were >>>> predefined, which is customarily applicable regarding this kind of >>>> functionality. Not every users having access should be able to create >>>> budget request, or perform subsequent actions. >>>> >>>> Lacking this, the user can (must?) select any partyId and roleId to >>>> associate with the budget request. Even parties not working with (or >>>> allowed to work with) OFBiz in general and the accounting module in >>>> particular can be selected. >>>> But also due to the lacking possibility to set the internal organisation, >>>> no default currency is set. Thus, making the amounts ambiguous to >>>> interpret. >>>> >>>> Furthermore it provides only a basic workflow process consisting of >>>> (approve, reject and review). Associating a party to a budget request >>>> doesn't warrant that others can not manipulate the content/data associated >>>> with the request. When I approved a budget request, I had the button to >>>> reject available. This shouldn't be possible. And at one occasion I could >>>> click the 'reject' button multipe time to get to the final state. >>>> While status changes are registered, it should also show who invoked the >>>> status change. >>>> >>>> Due to the fact that arbitrarily any party can be associated to a budget >>>> request without any restrictions, the workflow is rendered meaningless. >>>> >>>> Conclusion(s): >>>> This solutions hasn't been thought through properly and does no good for >>>> the quality of OFBiz (both the project and the product) in general and for >>>> the accounting module in particular. Prior to dumping the code into trunk >>>> it should have been posted for review by the community, so this could have >>>> been avoided. >>>> >>>> If any other contributor to this project made such a set of >>>> functionalities >>>> available as a patch to an OFBiz issue in JIRA, it would sit there for a >>>> great length of time before it would have been committed. >>>> >>>> I advice to remove it from trunk. >>>> >>>> Regards, >>>> >>>> Pierre Smits >>>> >>>> *ORRTIZ.COM <http://www.orrtiz.com>* >>>> >>>> Services & Solutions for Cloud- >>>> Based Manufacturing, Professional >>>> Services and Retail & Trade >>>> http://www.orrtiz.com >>>> >>>> > |
Administrator
|
In reply to this post by Pierre Smits
I think Sharan would not have included it in her book if it was in R13.07. I guess just a note to say it's not yet ready.
There are some guidelines in your 1st mail which could probe useful later, better to see the positive aspects ;) Jacques Le 10/08/2014 00:24, Pierre Smits a écrit : > To clarify: I didn't demand that this code was removed. I advised to > removed it. > > Pierre Smits > > *ORRTIZ.COM <http://www.orrtiz.com>* > Services & Solutions for Cloud- > Based Manufacturing, Professional > Services and Retail & Trade > http://www.orrtiz.com > > > On Fri, Jun 20, 2014 at 6:32 PM, Adrian Crum < > [hidden email]> wrote: > >> To clarify: I wasn't trying to push for a particular methodology. I was >> trying to address Pierre's approach to this community, where he demands >> that any code he doesn't like be reverted (he has done this before). >> Traditionally, that is not how we do things in this community. >> >> I agree that a commit that breaks the build process or severely impacts >> the operation of the application should be fixed as quickly as possible or >> it should be reverted. That is not the case in this thread. The new >> functionality might not meet Pierre's standards, but it isn't breaking the >> build. Therefore, I don't agree that it should be reverted. >> >> >> Adrian Crum >> Sandglass Software >> www.sandglass-software.com >> >> On 6/20/2014 9:01 AM, Adam Heath wrote: >> >>> On 06/20/2014 10:42 AM, Adrian Crum wrote: >>> >>>> No one is lowering the bar. The problem is, you still don't understand >>>> how an open source community works. >>>> >>> This is wrong. There is no hard-set one-workflow-to-bind-them >>> methodology here. Some people seem to think there is. >>> >>> The Review-Commit workflow people sometimes want all stuff reviewed. >>> While that can help find many issues ahead of time, it will never be >>> able to find and implement all corner cases. That can generally only >>> happen when the isolated code is pushed into the wider community. At >>> some point, you just need to push your reviewed code to someone else, >>> and continue to fix/improve afterwards. >>> >>> The Commit-Review workflow can sometimes cause severe breakage for other >>> users. If not-ready code is pushed into trunk, and it causes feature >>> breakage, or compilation problems, or corruption, the community at large >>> might not be able to help, as the newly pushed code is unknown, and >>> it'll take a while to come up to speed. >>> >>> Speaking about one earlier point: Committing code *early* does not lower >>> the bar for the committed code. Code is code. It is, or it isn't. When >>> something is committed has no bearing on the quality of the commit. >>> That quality is a separate item from the time of the commit. >>> >>> Let me give you an example: >>>> I helped design the custom XML file format OFBiz uses for UI labels >>>> (https://issues.apache.org/jira/browse/OFBIZ-1442). There were people >>>> in the community who disagreed with the design, but it was committed >>>> anyway. >>>> >>> Even here, you didn't do this initial work *in trunk*. You thought >>> about the idea for a while, tried some things, got an initial set, then >>> eventually committed to trunk. >>> >>> For the next few months, there were a lot of commits to fix bugs that >>>> cropped up after the initial commit. Scott and David helped with the >>>> bug fixes and improvements. >>>> >>>> Eventually, the new feature was working well - but there were still >>>> hundreds of properties files that needed to be converted to the new >>>> format. That was done over a period of several years by many different >>>> people. >>>> >>> Another concrete example. >>> >>> Currently, I'm working on fixes to the entityengine crypt system. >>> >>> 2 years ago, I implemented key-encrypting-key support(read up on the >>> credit card number security docs). I worked on the code in isolation, >>> on behalf of a customer. Once it worked there, I then directly pushed >>> to ofbiz trunk. This is the Commit-Review workflow. >>> >>> No review happened. None. If such review had happened, it might have >>> discovered that direct lookup of encrypted fields would have been broken >>> by my new code(a random salt is prepended to each encrypted value, which >>> prevents lookups). >>> >>> Even if that review *had* happened, after the commit, or even *before* >>> the commit, and I didn't add that random salt, it *still* wouldn't have >>> fixed the direct lookup problem. This was due to direct-lookup being >>> broken as far back as release4.0, and probably even all the way back to >>> 2005(!). This points to a general lack of review, at *all* levels. >>> >>> It's been my experience, that the Review-Commit workflow will get you >>> some small about of early reviews; those people who are keenly >>> interested in your new idea will possibly wake up and comment on it. >>> However, the Commit-Review can get you *more* reviewers; in addition to >>> those who are interested in the new stuff, you'll find completely random >>> people might speak up, and offer some un-thought-of problem. Even >>> simple things, like a null pointer check happening after a dereference >>> can be found this way. >>> |
Administrator
|
Also I forgot to mention (again) that, with our new policy to automatically grab new features or bug fixes from Jira, a direct commit should not
happen for major new features or bug fixes. We shall first create a Jira issue, even we you don't submit a patch (which is preferable for peers reviews), in order to fill the version to allow creating reports like https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel Jacques Le 10/08/2014 12:28, Jacques Le Roux a écrit : > I think Sharan would not have included it in her book if it was in R13.07. I guess just a note to say it's not yet ready. > > There are some guidelines in your 1st mail which could probe useful later, better to see the positive aspects ;) > > Jacques > > Le 10/08/2014 00:24, Pierre Smits a écrit : >> To clarify: I didn't demand that this code was removed. I advised to >> removed it. >> >> Pierre Smits >> >> *ORRTIZ.COM <http://www.orrtiz.com>* >> Services & Solutions for Cloud- >> Based Manufacturing, Professional >> Services and Retail & Trade >> http://www.orrtiz.com >> >> >> On Fri, Jun 20, 2014 at 6:32 PM, Adrian Crum < >> [hidden email]> wrote: >> >>> To clarify: I wasn't trying to push for a particular methodology. I was >>> trying to address Pierre's approach to this community, where he demands >>> that any code he doesn't like be reverted (he has done this before). >>> Traditionally, that is not how we do things in this community. >>> >>> I agree that a commit that breaks the build process or severely impacts >>> the operation of the application should be fixed as quickly as possible or >>> it should be reverted. That is not the case in this thread. The new >>> functionality might not meet Pierre's standards, but it isn't breaking the >>> build. Therefore, I don't agree that it should be reverted. >>> >>> >>> Adrian Crum >>> Sandglass Software >>> www.sandglass-software.com >>> >>> On 6/20/2014 9:01 AM, Adam Heath wrote: >>> >>>> On 06/20/2014 10:42 AM, Adrian Crum wrote: >>>> >>>>> No one is lowering the bar. The problem is, you still don't understand >>>>> how an open source community works. >>>>> >>>> This is wrong. There is no hard-set one-workflow-to-bind-them >>>> methodology here. Some people seem to think there is. >>>> >>>> The Review-Commit workflow people sometimes want all stuff reviewed. >>>> While that can help find many issues ahead of time, it will never be >>>> able to find and implement all corner cases. That can generally only >>>> happen when the isolated code is pushed into the wider community. At >>>> some point, you just need to push your reviewed code to someone else, >>>> and continue to fix/improve afterwards. >>>> >>>> The Commit-Review workflow can sometimes cause severe breakage for other >>>> users. If not-ready code is pushed into trunk, and it causes feature >>>> breakage, or compilation problems, or corruption, the community at large >>>> might not be able to help, as the newly pushed code is unknown, and >>>> it'll take a while to come up to speed. >>>> >>>> Speaking about one earlier point: Committing code *early* does not lower >>>> the bar for the committed code. Code is code. It is, or it isn't. When >>>> something is committed has no bearing on the quality of the commit. >>>> That quality is a separate item from the time of the commit. >>>> >>>> Let me give you an example: >>>>> I helped design the custom XML file format OFBiz uses for UI labels >>>>> (https://issues.apache.org/jira/browse/OFBIZ-1442). There were people >>>>> in the community who disagreed with the design, but it was committed >>>>> anyway. >>>>> >>>> Even here, you didn't do this initial work *in trunk*. You thought >>>> about the idea for a while, tried some things, got an initial set, then >>>> eventually committed to trunk. >>>> >>>> For the next few months, there were a lot of commits to fix bugs that >>>>> cropped up after the initial commit. Scott and David helped with the >>>>> bug fixes and improvements. >>>>> >>>>> Eventually, the new feature was working well - but there were still >>>>> hundreds of properties files that needed to be converted to the new >>>>> format. That was done over a period of several years by many different >>>>> people. >>>>> >>>> Another concrete example. >>>> >>>> Currently, I'm working on fixes to the entityengine crypt system. >>>> >>>> 2 years ago, I implemented key-encrypting-key support(read up on the >>>> credit card number security docs). I worked on the code in isolation, >>>> on behalf of a customer. Once it worked there, I then directly pushed >>>> to ofbiz trunk. This is the Commit-Review workflow. >>>> >>>> No review happened. None. If such review had happened, it might have >>>> discovered that direct lookup of encrypted fields would have been broken >>>> by my new code(a random salt is prepended to each encrypted value, which >>>> prevents lookups). >>>> >>>> Even if that review *had* happened, after the commit, or even *before* >>>> the commit, and I didn't add that random salt, it *still* wouldn't have >>>> fixed the direct lookup problem. This was due to direct-lookup being >>>> broken as far back as release4.0, and probably even all the way back to >>>> 2005(!). This points to a general lack of review, at *all* levels. >>>> >>>> It's been my experience, that the Review-Commit workflow will get you >>>> some small about of early reviews; those people who are keenly >>>> interested in your new idea will possibly wake up and comment on it. >>>> However, the Commit-Review can get you *more* reviewers; in addition to >>>> those who are interested in the new stuff, you'll find completely random >>>> people might speak up, and offer some un-thought-of problem. Even >>>> simple things, like a null pointer check happening after a dereference >>>> can be found this way. >>>> > |
Administrator
|
As mentioned Jacopo, the Jira issue creation can be done after, even by another person.
The point is to collect changes by versions (releases) Jacques Le 10/08/2014 14:14, Jacques Le Roux a écrit : > Also I forgot to mention (again) that, with our new policy to automatically grab new features or bug fixes from Jira, a direct commit should not > happen for major new features or bug fixes. > We shall first create a Jira issue, even we you don't submit a patch (which is preferable for peers reviews), in order to fill the version to allow > creating reports like > https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel > > Jacques > > Le 10/08/2014 12:28, Jacques Le Roux a écrit : >> I think Sharan would not have included it in her book if it was in R13.07. I guess just a note to say it's not yet ready. >> >> There are some guidelines in your 1st mail which could probe useful later, better to see the positive aspects ;) >> >> Jacques >> >> Le 10/08/2014 00:24, Pierre Smits a écrit : >>> To clarify: I didn't demand that this code was removed. I advised to >>> removed it. >>> >>> Pierre Smits >>> >>> *ORRTIZ.COM <http://www.orrtiz.com>* >>> Services & Solutions for Cloud- >>> Based Manufacturing, Professional >>> Services and Retail & Trade >>> http://www.orrtiz.com >>> >>> >>> On Fri, Jun 20, 2014 at 6:32 PM, Adrian Crum < >>> [hidden email]> wrote: >>> >>>> To clarify: I wasn't trying to push for a particular methodology. I was >>>> trying to address Pierre's approach to this community, where he demands >>>> that any code he doesn't like be reverted (he has done this before). >>>> Traditionally, that is not how we do things in this community. >>>> >>>> I agree that a commit that breaks the build process or severely impacts >>>> the operation of the application should be fixed as quickly as possible or >>>> it should be reverted. That is not the case in this thread. The new >>>> functionality might not meet Pierre's standards, but it isn't breaking the >>>> build. Therefore, I don't agree that it should be reverted. >>>> >>>> >>>> Adrian Crum >>>> Sandglass Software >>>> www.sandglass-software.com >>>> >>>> On 6/20/2014 9:01 AM, Adam Heath wrote: >>>> >>>>> On 06/20/2014 10:42 AM, Adrian Crum wrote: >>>>> >>>>>> No one is lowering the bar. The problem is, you still don't understand >>>>>> how an open source community works. >>>>>> >>>>> This is wrong. There is no hard-set one-workflow-to-bind-them >>>>> methodology here. Some people seem to think there is. >>>>> >>>>> The Review-Commit workflow people sometimes want all stuff reviewed. >>>>> While that can help find many issues ahead of time, it will never be >>>>> able to find and implement all corner cases. That can generally only >>>>> happen when the isolated code is pushed into the wider community. At >>>>> some point, you just need to push your reviewed code to someone else, >>>>> and continue to fix/improve afterwards. >>>>> >>>>> The Commit-Review workflow can sometimes cause severe breakage for other >>>>> users. If not-ready code is pushed into trunk, and it causes feature >>>>> breakage, or compilation problems, or corruption, the community at large >>>>> might not be able to help, as the newly pushed code is unknown, and >>>>> it'll take a while to come up to speed. >>>>> >>>>> Speaking about one earlier point: Committing code *early* does not lower >>>>> the bar for the committed code. Code is code. It is, or it isn't. When >>>>> something is committed has no bearing on the quality of the commit. >>>>> That quality is a separate item from the time of the commit. >>>>> >>>>> Let me give you an example: >>>>>> I helped design the custom XML file format OFBiz uses for UI labels >>>>>> (https://issues.apache.org/jira/browse/OFBIZ-1442). There were people >>>>>> in the community who disagreed with the design, but it was committed >>>>>> anyway. >>>>>> >>>>> Even here, you didn't do this initial work *in trunk*. You thought >>>>> about the idea for a while, tried some things, got an initial set, then >>>>> eventually committed to trunk. >>>>> >>>>> For the next few months, there were a lot of commits to fix bugs that >>>>>> cropped up after the initial commit. Scott and David helped with the >>>>>> bug fixes and improvements. >>>>>> >>>>>> Eventually, the new feature was working well - but there were still >>>>>> hundreds of properties files that needed to be converted to the new >>>>>> format. That was done over a period of several years by many different >>>>>> people. >>>>>> >>>>> Another concrete example. >>>>> >>>>> Currently, I'm working on fixes to the entityengine crypt system. >>>>> >>>>> 2 years ago, I implemented key-encrypting-key support(read up on the >>>>> credit card number security docs). I worked on the code in isolation, >>>>> on behalf of a customer. Once it worked there, I then directly pushed >>>>> to ofbiz trunk. This is the Commit-Review workflow. >>>>> >>>>> No review happened. None. If such review had happened, it might have >>>>> discovered that direct lookup of encrypted fields would have been broken >>>>> by my new code(a random salt is prepended to each encrypted value, which >>>>> prevents lookups). >>>>> >>>>> Even if that review *had* happened, after the commit, or even *before* >>>>> the commit, and I didn't add that random salt, it *still* wouldn't have >>>>> fixed the direct lookup problem. This was due to direct-lookup being >>>>> broken as far back as release4.0, and probably even all the way back to >>>>> 2005(!). This points to a general lack of review, at *all* levels. >>>>> >>>>> It's been my experience, that the Review-Commit workflow will get you >>>>> some small about of early reviews; those people who are keenly >>>>> interested in your new idea will possibly wake up and comment on it. >>>>> However, the Commit-Review can get you *more* reviewers; in addition to >>>>> those who are interested in the new stuff, you'll find completely random >>>>> people might speak up, and offer some un-thought-of problem. Even >>>>> simple things, like a null pointer check happening after a dereference >>>>> can be found this way. >>>>> >> > |
On 08/10/2014 08:19 AM, Jacques Le Roux wrote:
> As mentioned Jacopo, the Jira issue creation can be done after, even > by another person. > The point is to collect changes by versions (releases) > > Jacques > > Le 10/08/2014 14:14, Jacques Le Roux a écrit : >> Also I forgot to mention (again) that, with our new policy to >> automatically grab new features or bug fixes from Jira, a direct >> commit should not happen for major new features or bug fixes. >> We shall first create a Jira issue, even we you don't submit a patch >> (which is preferable for peers reviews), in order to fill the version >> to allow creating reports like >> https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel >> >> Oops, missed this, my bad, on my recent parallelization changes. ps: I really wish that there was some kind of git-based workflow. |
Administrator
|
You can still create a Jira issue and select the concerned versions
Jacques Le 13/08/2014 21:54, Adam Heath a écrit : > On 08/10/2014 08:19 AM, Jacques Le Roux wrote: >> As mentioned Jacopo, the Jira issue creation can be done after, even by another person. >> The point is to collect changes by versions (releases) >> >> Jacques >> >> Le 10/08/2014 14:14, Jacques Le Roux a écrit : >>> Also I forgot to mention (again) that, with our new policy to automatically grab new features or bug fixes from Jira, a direct commit should not >>> happen for major new features or bug fixes. >>> We shall first create a Jira issue, even we you don't submit a patch (which is preferable for peers reviews), in order to fill the version to >>> allow creating reports like >>> https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel >>> > > Oops, missed this, my bad, on my recent parallelization changes. > > ps: I really wish that there was some kind of git-based workflow. > > > |
Well, all versions, as it is a new feature. I developed it directly
against trunk, not any of a number of older deployed versions. I've got another change I've been working on, that I'll do this new way. Basically, Start.java will now take over stdout/stderr, run it through a TeeOutputStream, with output being sent do the original stdout/stderr, and to a file, then after full startup, the original stdout/stderr is turned off. Whenever the output file gets to be a certain size, or midnight happens, then the file is auto-rotated, and $last_file+1 is auto-compressed. I wrote this against trunk as well, in response to java 1.7 running on debian wheezy i386, but on top of xen, not working with 2G console.log files. The key difference to this is the "on top of xen". Without this patch, ofbiz would need to be restarted externally, and the log rotated. With the patch(es), it stays up basically forever. On 08/13/2014 03:21 PM, Jacques Le Roux wrote: > You can still create a Jira issue and select the concerned versions > > Jacques > > Le 13/08/2014 21:54, Adam Heath a écrit : >> On 08/10/2014 08:19 AM, Jacques Le Roux wrote: >>> As mentioned Jacopo, the Jira issue creation can be done after, even >>> by another person. >>> The point is to collect changes by versions (releases) >>> >>> Jacques >>> >>> Le 10/08/2014 14:14, Jacques Le Roux a écrit : >>>> Also I forgot to mention (again) that, with our new policy to >>>> automatically grab new features or bug fixes from Jira, a direct >>>> commit should not happen for major new features or bug fixes. >>>> We shall first create a Jira issue, even we you don't submit a >>>> patch (which is preferable for peers reviews), in order to fill the >>>> version to allow creating reports like >>>> https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel >>>> >>>> >> >> Oops, missed this, my bad, on my recent parallelization changes. >> >> ps: I really wish that there was some kind of git-based workflow. >> >> >> |
Btw, we(Brainfood) have been using
gitlab(https://about.gitlab.com/gitlab-ce/), which has the MIT license; I wonder if we could get a copy of that installed, because I just don't like the commercial github. On 08/13/2014 06:17 PM, Adam Heath wrote: > Well, all versions, as it is a new feature. I developed it directly > against trunk, not any of a number of older deployed versions. > > I've got another change I've been working on, that I'll do this new > way. Basically, Start.java will now take over stdout/stderr, run it > through a TeeOutputStream, with output being sent do the original > stdout/stderr, and to a file, then after full startup, the original > stdout/stderr is turned off. Whenever the output file gets to be a > certain size, or midnight happens, then the file is auto-rotated, and > $last_file+1 is auto-compressed. > > I wrote this against trunk as well, in response to java 1.7 running on > debian wheezy i386, but on top of xen, not working with 2G console.log > files. The key difference to this is the "on top of xen". > > Without this patch, ofbiz would need to be restarted externally, and > the log rotated. With the patch(es), it stays up basically forever. > > On 08/13/2014 03:21 PM, Jacques Le Roux wrote: >> You can still create a Jira issue and select the concerned versions >> >> Jacques >> >> Le 13/08/2014 21:54, Adam Heath a écrit : >>> On 08/10/2014 08:19 AM, Jacques Le Roux wrote: >>>> As mentioned Jacopo, the Jira issue creation can be done after, >>>> even by another person. >>>> The point is to collect changes by versions (releases) >>>> >>>> Jacques >>>> >>>> Le 10/08/2014 14:14, Jacques Le Roux a écrit : >>>>> Also I forgot to mention (again) that, with our new policy to >>>>> automatically grab new features or bug fixes from Jira, a direct >>>>> commit should not happen for major new features or bug fixes. >>>>> We shall first create a Jira issue, even we you don't submit a >>>>> patch (which is preferable for peers reviews), in order to fill >>>>> the version to allow creating reports like >>>>> https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel >>>>> >>>>> >>> >>> Oops, missed this, my bad, on my recent parallelization changes. >>> >>> ps: I really wish that there was some kind of git-based workflow. >>> >>> >>> > |
Administrator
|
In reply to this post by Adam Heath-2
Le 14/08/2014 01:17, Adam Heath a écrit : > Well, all versions, as it is a new feature. I developed it directly against trunk, not any of a number of older deployed versions. For trunk select rather "Upcoming Branch" Jacques > > I've got another change I've been working on, that I'll do this new way. Basically, Start.java will now take over stdout/stderr, run it through a > TeeOutputStream, with output being sent do the original stdout/stderr, and to a file, then after full startup, the original stdout/stderr is turned > off. Whenever the output file gets to be a certain size, or midnight happens, then the file is auto-rotated, and $last_file+1 is auto-compressed. > > I wrote this against trunk as well, in response to java 1.7 running on debian wheezy i386, but on top of xen, not working with 2G console.log > files. The key difference to this is the "on top of xen". > > Without this patch, ofbiz would need to be restarted externally, and the log rotated. With the patch(es), it stays up basically forever. > > On 08/13/2014 03:21 PM, Jacques Le Roux wrote: >> You can still create a Jira issue and select the concerned versions >> >> Jacques >> >> Le 13/08/2014 21:54, Adam Heath a écrit : >>> On 08/10/2014 08:19 AM, Jacques Le Roux wrote: >>>> As mentioned Jacopo, the Jira issue creation can be done after, even by another person. >>>> The point is to collect changes by versions (releases) >>>> >>>> Jacques >>>> >>>> Le 10/08/2014 14:14, Jacques Le Roux a écrit : >>>>> Also I forgot to mention (again) that, with our new policy to automatically grab new features or bug fixes from Jira, a direct commit should >>>>> not happen for major new features or bug fixes. >>>>> We shall first create a Jira issue, even we you don't submit a patch (which is preferable for peers reviews), in order to fill the version to >>>>> allow creating reports like >>>>> https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel >>>>> >>> >>> Oops, missed this, my bad, on my recent parallelization changes. >>> >>> ps: I really wish that there was some kind of git-based workflow. >>> >>> >>> > > |
Administrator
|
In reply to this post by Adam Heath-2
Le 14/08/2014 01:24, Adam Heath a écrit : > Btw, we(Brainfood) have been using gitlab(https://about.gitlab.com/gitlab-ce/), which has the MIT license; I wonder if we could get a copy of that > installed, because I just don't like the commercial github. You mean at the ASF? Jacques > > On 08/13/2014 06:17 PM, Adam Heath wrote: >> Well, all versions, as it is a new feature. I developed it directly against trunk, not any of a number of older deployed versions. >> >> I've got another change I've been working on, that I'll do this new way. Basically, Start.java will now take over stdout/stderr, run it through a >> TeeOutputStream, with output being sent do the original stdout/stderr, and to a file, then after full startup, the original stdout/stderr is turned >> off. Whenever the output file gets to be a certain size, or midnight happens, then the file is auto-rotated, and $last_file+1 is auto-compressed. >> >> I wrote this against trunk as well, in response to java 1.7 running on debian wheezy i386, but on top of xen, not working with 2G console.log >> files. The key difference to this is the "on top of xen". >> >> Without this patch, ofbiz would need to be restarted externally, and the log rotated. With the patch(es), it stays up basically forever. >> >> On 08/13/2014 03:21 PM, Jacques Le Roux wrote: >>> You can still create a Jira issue and select the concerned versions >>> >>> Jacques >>> >>> Le 13/08/2014 21:54, Adam Heath a écrit : >>>> On 08/10/2014 08:19 AM, Jacques Le Roux wrote: >>>>> As mentioned Jacopo, the Jira issue creation can be done after, even by another person. >>>>> The point is to collect changes by versions (releases) >>>>> >>>>> Jacques >>>>> >>>>> Le 10/08/2014 14:14, Jacques Le Roux a écrit : >>>>>> Also I forgot to mention (again) that, with our new policy to automatically grab new features or bug fixes from Jira, a direct commit should >>>>>> not happen for major new features or bug fixes. >>>>>> We shall first create a Jira issue, even we you don't submit a patch (which is preferable for peers reviews), in order to fill the version to >>>>>> allow creating reports like >>>>>> https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel >>>>>> >>>> >>>> Oops, missed this, my bad, on my recent parallelization changes. >>>> >>>> ps: I really wish that there was some kind of git-based workflow. >>>> >>>> >>>> >> > > |
Perhaps also good to know that we are currently implementing:
git-gerrit-jenkins-ofbizscrum of which the git security and peer review and ofbizscrum: ticket-backlog-timereg-invoicing is essential to us. Jira for us is a kind of sideline product, not part of the workflow, just sitting on the side line as a manual optional additional step which does not automate stuff, just adds extra work. Regards, Hans On 14/08/14 08:59, Jacques Le Roux wrote: > > Le 14/08/2014 01:24, Adam Heath a écrit : >> Btw, we(Brainfood) have been using >> gitlab(https://about.gitlab.com/gitlab-ce/), which has the MIT >> license; I wonder if we could get a copy of that installed, because I >> just don't like the commercial github. > > You mean at the ASF? > > Jacques > >> >> On 08/13/2014 06:17 PM, Adam Heath wrote: >>> Well, all versions, as it is a new feature. I developed it directly >>> against trunk, not any of a number of older deployed versions. >>> >>> I've got another change I've been working on, that I'll do this new >>> way. Basically, Start.java will now take over stdout/stderr, run it >>> through a TeeOutputStream, with output being sent do the original >>> stdout/stderr, and to a file, then after full startup, the original >>> stdout/stderr is turned off. Whenever the output file gets to be a >>> certain size, or midnight happens, then the file is auto-rotated, >>> and $last_file+1 is auto-compressed. >>> >>> I wrote this against trunk as well, in response to java 1.7 running >>> on debian wheezy i386, but on top of xen, not working with 2G >>> console.log files. The key difference to this is the "on top of xen". >>> >>> Without this patch, ofbiz would need to be restarted externally, and >>> the log rotated. With the patch(es), it stays up basically forever. >>> >>> On 08/13/2014 03:21 PM, Jacques Le Roux wrote: >>>> You can still create a Jira issue and select the concerned versions >>>> >>>> Jacques >>>> >>>> Le 13/08/2014 21:54, Adam Heath a écrit : >>>>> On 08/10/2014 08:19 AM, Jacques Le Roux wrote: >>>>>> As mentioned Jacopo, the Jira issue creation can be done after, >>>>>> even by another person. >>>>>> The point is to collect changes by versions (releases) >>>>>> >>>>>> Jacques >>>>>> >>>>>> Le 10/08/2014 14:14, Jacques Le Roux a écrit : >>>>>>> Also I forgot to mention (again) that, with our new policy to >>>>>>> automatically grab new features or bug fixes from Jira, a direct >>>>>>> commit should not happen for major new features or bug fixes. >>>>>>> We shall first create a Jira issue, even we you don't submit a >>>>>>> patch (which is preferable for peers reviews), in order to fill >>>>>>> the version to allow creating reports like >>>>>>> https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel >>>>>>> >>>>>>> >>>>> >>>>> Oops, missed this, my bad, on my recent parallelization changes. >>>>> >>>>> ps: I really wish that there was some kind of git-based workflow. >>>>> >>>>> >>>>> >>> >> >> |
Free forum by Nabble | Edit this page |