Re: [Maposmatic-dev] MapOSMatic daemon rewrite

[Thomas Petazzoni]

On Sun, 24 Jan 2010 14:43:14 +0100
Maxime Petazzoni <address@hidden> wrote:

>   [PATCH maposmatic 1/9] Improve the file cleanup mechanism
>   [PATCH maposmatic 2/9] Cornerstones for a new MapOSMatic daemon
>   [PATCH maposmatic 3/9] Update the .gitignore list
>   [PATCH maposmatic 4/9] Add MAPOSMATIC_LOG_LEVEL to the environment
> wrapping [PATCH maposmatic 5/9] Merge the StandaloneMapOSMaticDaemon
> into the base MapOSMaticDaemon class
>   [PATCH maposmatic 6/9] Revamp the job renderer
>   [PATCH maposmatic 7/9] Rework the daemon to use the new JobRenderers
>   [PATCH maposmatic 8/9] Frequency parameters passing improvement
>   [PATCH maposmatic 9/9] Provide a map_areas prefix to the
> TimingOutJobRenderer

Ok for me. Thanks a lot for your work!

A few remarks though:

 * The file cleanup mechanism should have been in a separate branch,
   because it's a separate issue. The file cleanup work needs to be
   quickly sorted out and deployed on the prod server, while the daemon
   rewrite is not as urgent and needs more testing at the dev.m.o level.

 * Your patch set is not easily reviewable because some patches changes
   choices made in previous patches. For example, patch 5/9 is really
   strange. Why haven't you done this in 2/9 from the beginning ? More
   or less the same for 6/9 and 7/9.

 * I find the thing a little bit too complicated: we don't need several
   scheduling policies, and I don't like having code that does not do
   anything (the ProcessingMapOSMaticDaemon and
   SubprocessMapOSMaticDaemon classes, see the
   http://en.wikipedia.org/wiki/You_ain%27t_gonna_need_it principle).
   However, these issues are fixable afterwards.

Can you merge this work and deploy on dev.m.o ?

Thanks again!

Thomas
-- 
Thomas Petazzoni                         http://thomas.enix.org
Promouvoir et défendre le Logiciel Libre http://www.april.org
Logiciels Libres à Toulouse              http://www.toulibre.org

signature.asc
Description: PGP signature