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 < [email protected]> 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. >> >
