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

[Carlo de Wolf]

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-separation-of-concern
>>>
>>> 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/658a23d43711e5b1f2dc8a3f7c5506364df9175d

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