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

[Carlo de Wolf]

An example of what I would consider design regression would be:
https://github.com/jbossas/jboss-as/blob/master/remoting/src/main/java/org/jboss/as/remoting/ChannelOpenListenerService.java

I came across this while working on AS7-423. The Remoting facility 
ChannelOpenListenerService now has a direct dependency on the management 
protocol. Which makes it unusable (not re-usable) for the EJB protocol.

Carlo
==
No external components were harmed during the creation of this mail. ;-)

On 08/26/2011 08:34 AM, Carlo de Wolf wrote:
> 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
>