Code review comment for lp:~linaro-infrastructure/linaro-ci-dashboard/sync-builds

Revision history for this message
Deepti B. Kalakeri (deeptik) wrote :

Ah! good amount of changes. Thanks for working on this.

some problems, that we need to fix

1) I tried merging your code with the upstream linaro-ci-dashboard, there are lot of merge conflicts, please rebase your changes on top of the latest linaro-ci-dashboard.

2) test_create_integration_job_xml test fails because your changes has renamed the function create_job_xml() to create_integration_job_xml.

3) The jenkins sync command only seems to update the job created using the IntegrationLoop/create url. What about the information of the jobs which are created outsider of the Loop, maybe using jenkins directly ?

4) Also, the values stored in the frontend.loopbuild table should match the actual build numbers on the jenkins. For ex: whenever we press the schedule New build we get a new build_number, this should in turn store job buildnumber in remote_buildnumber if the build job was successful. build_job returns the job details in the form of xml once it is successfully scheduled to run or a JenkinsException on failure. We should probably call get_job_info() and get the status of the job and accordingly the status of the job also needs to be updated on the detail page and in the database instead of hard coding.

review: Needs Fixing (code)

« Back to merge proposal