chris2fr / redmine_taskjuggler

Redmine and Task Juggler Integration
25 stars 9 forks source link

Refactorization of the project/task recursive algorithm #44

Closed guyzmo closed 9 years ago

guyzmo commented 9 years ago
chris2fr commented 9 years ago

I am merging without any formalism for now. I love the spirit of this work: enthusiasm for the future and realism for the cleaning up of code to be done. It reminds me of a work I kind of started on an Architecture branch without quite finishing it.

Here is the email I got around this (in French):

cf mon Pull Request sur github et mes comments sur IRC. J'ai modifié

l'algo complètement, et l'ai réduit à deux fonctions:

# Project hierarchy TaskJuggler creation
# project: redmine project instance
# parent: taskjuggler task instance (or nil)
# returns taskjuggler task instance
def redmine_project_to_taskjuggler_task(project, parent=nil)

# Task hierarchy TaskJuggler creation
# issue: redmine issue instance
# parent: taskjuggler task instance (built from project or issue)
# returns taskjuggler task instance
def redmine_issue_to_taskjuggler_task(issue, parent)

le  point  d'entrée  reste  le  même,  le  résultat   est  (presque)

identique, sauf qu'au lieu de retourner une liste je retourne une instance unique de Task.

Chacune des deux fonctions est récursive, et fonctionne comme suit:

- Build task from project
    - for each issue of project
        - if issue hasn't been seen
            - Build task from issue
    - for each sub-project of project
        - Build sub-task from sub-project
        - append sub-task to task
    - return task

- Build task from issue
    - for each sub-issue of issue
        - if sub-issue hasn't been seen
            - Build sub-task from sub-issue
            - add sub-task to task
    - return task

Piece of cake :-)

Maintenant, j'ai bougé la définition des paramètres de la  task dans

la fonction redmine_issue_taskjuggler_task() en ayant presque pas fait de modifications. J'ai peur que cette partie soit buggée, étant donné que je n'ai que des start/end qui n'apparaissent , sans obtenir de depends ou d'effort/length/duration.

A ce niveau là, je me demande s'il n'y a pas des questions de design

à se poser, notamment pourquoi avoir mis en place les timepoints, alors qu'il y a déjà les champs Start/Due/Length dans redmine ? La redondance est-elle vraiment nécessaire ?

De même pour l'allocation des utilisateurs,  ne  pourrions  nous pas

nous baser sur les users redmine directement (assignee!), qui ont déjà pas mal d'informations à apporter.

D'un point vu plus "technique", je pense que le  choix  de  faire la

conversion via les .to_s() est une fausse bonne idée. C'est principalement mon expérience du python (qui de façon similaire a la méthode __str__()), où j'ai remarqué qu'il valait mieux créer une méthode explicite du type build_tjp(), car la méthode to_s() est utilisée par le système (notamment pour le debug) et est pratique à garder. Ainsi, ça évite d'avoir des morceaux de fichiers tjp dans les logs, et rend les points de conversion plus lisibles.

Enfin, une fois qu'on aura nettoyé le code, je pense  vraiment qu'il

pourrait être malin de transformer le plugin en client vers l'API REST de redmine. Biensûr, il faudra garder un plugin pour les différents éléments du modèle qui manquent à redmine, et aussi peut-être pour étendre l'API avec les méthodes qui pourraient nous manquer. Mais on en reparlera quand on aura nettoyé et mis à jour le codebase actuel ;-) On va pas mettre la charue avant les boeux…

Cheers,