Re: [jboss-as7-dev] Moving EJB3 code into the AS7 source tree

On 08/26/2011 06:58 AM, Stuart Douglas wrote: > I have moved the code and tests over. > > Stuart > > On 26/08/2011, at 3:42 AM, Jason T. Greene wrote: > >> On 8/25/11 3:09 AM, Carlo de Wolf wrote: >>> http://community.jboss.org/people/wolfc/blog/2010/11/26/strategies-for-se... >>> >>> >>> The only objection I have is putting everything in a single module and >>> just separate concerns on a package level. >> There is also some logical separation in that ejb3 is still a >> separate subsystem, and therefore a separate module. The code is >> just in the same place and versioned with all other subsystems. I >> think it's fine to break things into separately versioned external >> components when we have reuse (or great potential of reuse). The >> ejb3 timer implementation though is unlikely to be useful to anyone >> other than our impl. So using package separation for that case seems >> more than adequate (at least now) I wasn't talking about an external component. So based on that false assumption the commit has been approved. I also see that we've lost the history of the source code, while git is capable of (and designed for) slicing and dicing code bases including history. I would rather go for an open minded approach which gives the freedom for re-use, then such a close minded one. But I agree with you that we are incapable of fostering such an environment. >>> As I've pointed out a couple of times, in the AS 7 code base we're not >>> vigilant enough nor does the review process catch design issues. >>> For the >>> review process to do a proper design check would form an unwanted >>> choke >>> point. So we still need to find ways to guard the code against design >>> regression. >> We certainly could improve here in various areas. We need to make >> sure we capture designs in wikis or in javadoc or in code comments. >> This is done in many cases but not others. Ideally we start with an >> agreed upon requirements doc, then a set of docs detailing the >> abstract design. Then when code is created we try to reflect the doc >> information in comments and javadoc. Any significant refactor then >> can be reviewed against that information. >> >> Right now we certainly discuss all major EE refactors, and we >> validate they introduce no test regressions and no TCK regressions >> before we merge these kind of patches. That works on the assumption that tests get written (and get written well). Something I recently encountered showed that we're still lacking coverage: https://github.com/wolfc/jboss-as/commit/658a23d43711e5b1f2dc8a3f7c550636... Note that I'm a guilty party here as well as I did not write a test-case to guard my initial MDB implementation (just a demo). >> As to preventing choke points, I think this has to do with what the >> patch touches. The burden is ultimately on the person sending in the >> pull request / patch to break up changes in such a way that they are >> applied in reasonable time frames. For example simple bug fixes >> require much less review than a complete re-architecture of EE proxies. Yes, pull requests creating a burden on the reviewer. That's the nature of our current review process. Whether a contribution is applied in a timely manner is up to the pushers. To go by example I've got a contribution that is a first stage of on-going work, yet it has been sitting idly for some time now: https://github.com/jbossas/jboss-as/pull/90 Carlo >> -- >> Jason T. Greene >> JBoss AS Lead / EAP Platform Architect >> JBoss, a division of Red Hat