Review on Distributed Builds

[Marica Tan <ma...@gmail.com>]

Hi,

We had a code review session on the distributed builds component and came up
with a list of changes and suggestions.

1. Remove the checking of the state of the SCM Result in the build project
phase since this is already checked in the prepare build phase
    - review and add a test if possible to make sure we didn't break
anything

2. Use an event listener when the build agent configuration is changed,
instead of calling reload()
    - still need to review if it's possible

3. In PrepareBuildProjectTaskExecutor#executeTask(...), fix the checking of
SCM root to because it's hard to understand having a lot of ! in the
conditions.

4. In DefaultDistributedBuildManager, add a comment or rename the method
getOverallDistributedBuildQueryByGroup(...) so it would be easier to
understand what it is doing or what's going on inside the method.

5. Review how data is passed from client to server.  There might be a better
way than using a build context. Also some of the keys are duplicated and
there are different sets of keys defined for the master and for the build
agent.

6. Review the checkout and build process code of the build agent and see if
it can be used for the same actions in the core to eliminate the duplicate
actions being performed

7. Fixed CONTINUUM-2517 - project should not be queued to a build agent
while it is currently building there


Any comments or suggestions?


Thanks,
--
Marica

Re: Review on Distributed Builds

[Wendy Smoak <ws...@gmail.com>]

Thanks for spending time looking at this!

On Tue, May 4, 2010 at 9:10 PM, Marica Tan <ma...@gmail.com> wrote:
> 7. Fixed CONTINUUM-2517 - project should not be queued to a build agent
> while it is currently building there
>
> Any comments or suggestions?

I commented on the issue for 7, it's probably easiest to continue the
discussion there.

-- 
Wendy