----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/36/ -----------------------------------------------------------
Review request for blockerbugs.
Repository: blockerbugs
Description -------
Add partial updates sync. Display proposed/accepted fe/blocker bugs list associated with update on update list web page.
Diffs -----
testing/testfunc_update_sync.py 69f6bf8a459c71a9325cd633e7e33ce29354d7cd testing/test_updatesync_extract_information.py 1fc701ef79a74e27cf8223b56a04d63025b6509f testing/test_controllers.py f351832ad4350fcff3d67c0137ff62892639e908 blockerbugs/util/update_sync.py d664839ec1c5979dce980e7baad58154f4622e11 blockerbugs/util/bug_sync.py 49cce49740cd6f5b1f430f58c8d1b522e1f0b7e3 blockerbugs/templates/update_list.html a83dd9a422e51ea58e04ae58acd8fefe6eb1f79f blockerbugs/models/update.py 2ee3ca7ee62bd55bbd71555c13fecfab504c4f07 blockerbugs/controllers/main.py 4f23e35558ac3f8ed2ccc5d897b2219e449e7277 blockerbugs/cli.py 694736d098b6285c53db91f91350360d478c4229
Diff: http://reviewboard-tflink.rhcloud.com/r/36/diff/
Testing -------
Wrote test suites. I've tested on my develop instance.
Thanks,
Ilgiz Islamgulov
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/36/#review42 -----------------------------------------------------------
blockerbugs/controllers/main.py http://reviewboard-tflink.rhcloud.com/r/36/#comment69
this query has become rather complicated, could you add a quick comment about what it's doing?
blockerbugs/controllers/main.py http://reviewboard-tflink.rhcloud.com/r/36/#comment70
A short comment on this query would also be nice
blockerbugs/templates/update_list.html http://reviewboard-tflink.rhcloud.com/r/36/#comment73
I'd rather see these two tables be collapsed into a loop instead of having the code duplication
blockerbugs/templates/update_list.html http://reviewboard-tflink.rhcloud.com/r/36/#comment71
This needs another empty <td></td> - you caught the other one, though
blockerbugs/templates/update_list.html http://reviewboard-tflink.rhcloud.com/r/36/#comment72
you need quotes around the {{ bug.summary }} or else only the first word of the title will show up in the tooltip
A couple of minor nits but overall it looks good.
- Tim Flink
On July 15, 2013, 5:18 p.m., Ilgiz Islamgulov wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/36/
(Updated July 15, 2013, 5:18 p.m.)
Review request for blockerbugs.
Repository: blockerbugs
Description
Add partial updates sync. Display proposed/accepted fe/blocker bugs list associated with update on update list web page.
Diffs
testing/testfunc_update_sync.py 69f6bf8a459c71a9325cd633e7e33ce29354d7cd testing/test_updatesync_extract_information.py 1fc701ef79a74e27cf8223b56a04d63025b6509f testing/test_controllers.py f351832ad4350fcff3d67c0137ff62892639e908 blockerbugs/util/update_sync.py d664839ec1c5979dce980e7baad58154f4622e11 blockerbugs/util/bug_sync.py 49cce49740cd6f5b1f430f58c8d1b522e1f0b7e3 blockerbugs/templates/update_list.html a83dd9a422e51ea58e04ae58acd8fefe6eb1f79f blockerbugs/models/update.py 2ee3ca7ee62bd55bbd71555c13fecfab504c4f07 blockerbugs/controllers/main.py 4f23e35558ac3f8ed2ccc5d897b2219e449e7277 blockerbugs/cli.py 694736d098b6285c53db91f91350360d478c4229
Diff: http://reviewboard-tflink.rhcloud.com/r/36/diff/
Testing
Wrote test suites. I've tested on my develop instance.
Thanks,
Ilgiz Islamgulov
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/36/ -----------------------------------------------------------
(Updated July 19, 2013, 7:05 p.m.)
Review request for blockerbugs.
Repository: blockerbugs
Description -------
Add partial updates sync. Display proposed/accepted fe/blocker bugs list associated with update on update list web page.
Diffs (updated) -----
testing/testfunc_update_sync.py 69f6bf8a459c71a9325cd633e7e33ce29354d7cd testing/test_updatesync_extract_information.py 1fc701ef79a74e27cf8223b56a04d63025b6509f testing/test_controllers.py f351832ad4350fcff3d67c0137ff62892639e908 blockerbugs/util/update_sync.py d664839ec1c5979dce980e7baad58154f4622e11 blockerbugs/util/bug_sync.py 49cce49740cd6f5b1f430f58c8d1b522e1f0b7e3 blockerbugs/templates/update_list.html a83dd9a422e51ea58e04ae58acd8fefe6eb1f79f blockerbugs/models/update.py 2ee3ca7ee62bd55bbd71555c13fecfab504c4f07 blockerbugs/controllers/main.py 4f23e35558ac3f8ed2ccc5d897b2219e449e7277 blockerbugs/cli.py 694736d098b6285c53db91f91350360d478c4229
Diff: http://reviewboard-tflink.rhcloud.com/r/36/diff/
Testing -------
Wrote test suites. I've tested on my develop instance.
Thanks,
Ilgiz Islamgulov
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/36/#review47 -----------------------------------------------------------
blockerbugs/templates/update_list.html http://reviewboard-tflink.rhcloud.com/r/36/#comment74
got it, thank you
- Ilgiz Islamgulov
On July 19, 2013, 7:05 p.m., Ilgiz Islamgulov wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/36/
(Updated July 19, 2013, 7:05 p.m.)
Review request for blockerbugs.
Repository: blockerbugs
Description
Add partial updates sync. Display proposed/accepted fe/blocker bugs list associated with update on update list web page.
Diffs
testing/testfunc_update_sync.py 69f6bf8a459c71a9325cd633e7e33ce29354d7cd testing/test_updatesync_extract_information.py 1fc701ef79a74e27cf8223b56a04d63025b6509f testing/test_controllers.py f351832ad4350fcff3d67c0137ff62892639e908 blockerbugs/util/update_sync.py d664839ec1c5979dce980e7baad58154f4622e11 blockerbugs/util/bug_sync.py 49cce49740cd6f5b1f430f58c8d1b522e1f0b7e3 blockerbugs/templates/update_list.html a83dd9a422e51ea58e04ae58acd8fefe6eb1f79f blockerbugs/models/update.py 2ee3ca7ee62bd55bbd71555c13fecfab504c4f07 blockerbugs/controllers/main.py 4f23e35558ac3f8ed2ccc5d897b2219e449e7277 blockerbugs/cli.py 694736d098b6285c53db91f91350360d478c4229
Diff: http://reviewboard-tflink.rhcloud.com/r/36/diff/
Testing
Wrote test suites. I've tested on my develop instance.
Thanks,
Ilgiz Islamgulov
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/36/#review57 -----------------------------------------------------------
blockerbugs/controllers/main.py http://reviewboard-tflink.rhcloud.com/r/36/#comment88
minor phrasing nit: which are 'pending stable' in bodhi and are marked as fixing any non-rejected blocker/fe bug
blockerbugs/controllers/main.py http://reviewboard-tflink.rhcloud.com/r/36/#comment89
similar phrasing nit: which are 'updates testing' in bodhi (not updates testing pending) and are marked as fixing any non-rejected blocker/fe bug
blockerbugs/util/update_sync.py http://reviewboard-tflink.rhcloud.com/r/36/#comment90
I don't understand the reason for this change - it seems purely stylistic
blockerbugs/util/update_sync.py http://reviewboard-tflink.rhcloud.com/r/36/#comment91
good catch
testing/testfunc_update_sync.py http://reviewboard-tflink.rhcloud.com/r/36/#comment92
I'd rather keep these on separate lines - it make the data much easier to read
testing/testfunc_update_sync.py http://reviewboard-tflink.rhcloud.com/r/36/#comment93
I don't understand why all this update data is being defined statically - couldn't you get the same effect by generating almost identical updates and just changing the bits that you need to change?
Found a few other things, looks good functionally, though. I think most of this is minor stuff
- Tim Flink
On July 19, 2013, 7:05 p.m., Ilgiz Islamgulov wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/36/
(Updated July 19, 2013, 7:05 p.m.)
Review request for blockerbugs.
Repository: blockerbugs
Description
Add partial updates sync. Display proposed/accepted fe/blocker bugs list associated with update on update list web page.
Diffs
testing/testfunc_update_sync.py 69f6bf8a459c71a9325cd633e7e33ce29354d7cd testing/test_updatesync_extract_information.py 1fc701ef79a74e27cf8223b56a04d63025b6509f testing/test_controllers.py f351832ad4350fcff3d67c0137ff62892639e908 blockerbugs/util/update_sync.py d664839ec1c5979dce980e7baad58154f4622e11 blockerbugs/util/bug_sync.py 49cce49740cd6f5b1f430f58c8d1b522e1f0b7e3 blockerbugs/templates/update_list.html a83dd9a422e51ea58e04ae58acd8fefe6eb1f79f blockerbugs/models/update.py 2ee3ca7ee62bd55bbd71555c13fecfab504c4f07 blockerbugs/controllers/main.py 4f23e35558ac3f8ed2ccc5d897b2219e449e7277 blockerbugs/cli.py 694736d098b6285c53db91f91350360d478c4229
Diff: http://reviewboard-tflink.rhcloud.com/r/36/diff/
Testing
Wrote test suites. I've tested on my develop instance.
Thanks,
Ilgiz Islamgulov
On Aug. 2, 2013, 2:58 p.m., Tim Flink wrote:
blockerbugs/util/update_sync.py, line 40 http://reviewboard-tflink.rhcloud.com/r/36/diff/2/?file=525#file525line40
I don't understand the reason for this change - it seems purely stylistic
yes, it's stylistic only changes. As for me `if cond: ... else ...` Is more explicit than: `if not cond: ... else ... `
On Aug. 2, 2013, 2:58 p.m., Tim Flink wrote:
testing/testfunc_update_sync.py, line 49 http://reviewboard-tflink.rhcloud.com/r/36/diff/2/?file=528#file528line49
I don't understand why all this update data is being defined statically - couldn't you get the same effect by generating almost identical updates and just changing the bits that you need to change?
Indeed, I can extract a common part.
- Ilgiz
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/36/#review57 -----------------------------------------------------------
On July 19, 2013, 7:05 p.m., Ilgiz Islamgulov wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/36/
(Updated July 19, 2013, 7:05 p.m.)
Review request for blockerbugs.
Repository: blockerbugs
Description
Add partial updates sync. Display proposed/accepted fe/blocker bugs list associated with update on update list web page.
Diffs
testing/testfunc_update_sync.py 69f6bf8a459c71a9325cd633e7e33ce29354d7cd testing/test_updatesync_extract_information.py 1fc701ef79a74e27cf8223b56a04d63025b6509f testing/test_controllers.py f351832ad4350fcff3d67c0137ff62892639e908 blockerbugs/util/update_sync.py d664839ec1c5979dce980e7baad58154f4622e11 blockerbugs/util/bug_sync.py 49cce49740cd6f5b1f430f58c8d1b522e1f0b7e3 blockerbugs/templates/update_list.html a83dd9a422e51ea58e04ae58acd8fefe6eb1f79f blockerbugs/models/update.py 2ee3ca7ee62bd55bbd71555c13fecfab504c4f07 blockerbugs/controllers/main.py 4f23e35558ac3f8ed2ccc5d897b2219e449e7277 blockerbugs/cli.py 694736d098b6285c53db91f91350360d478c4229
Diff: http://reviewboard-tflink.rhcloud.com/r/36/diff/
Testing
Wrote test suites. I've tested on my develop instance.
Thanks,
Ilgiz Islamgulov
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/36/ -----------------------------------------------------------
(Updated Aug. 5, 2013, 12:32 p.m.)
Review request for blockerbugs.
Changes -------
Repository: blockerbugs
Description -------
Add partial updates sync. Display proposed/accepted fe/blocker bugs list associated with update on update list web page.
Diffs (updated) -----
testing/testfunc_update_sync.py 69f6bf8a459c71a9325cd633e7e33ce29354d7cd testing/test_updatesync_extract_information.py 1fc701ef79a74e27cf8223b56a04d63025b6509f testing/test_controllers.py f351832ad4350fcff3d67c0137ff62892639e908 blockerbugs/util/update_sync.py d664839ec1c5979dce980e7baad58154f4622e11 blockerbugs/util/bug_sync.py 49cce49740cd6f5b1f430f58c8d1b522e1f0b7e3 blockerbugs/templates/update_list.html a83dd9a422e51ea58e04ae58acd8fefe6eb1f79f blockerbugs/models/update.py 2ee3ca7ee62bd55bbd71555c13fecfab504c4f07 blockerbugs/controllers/main.py 4f23e35558ac3f8ed2ccc5d897b2219e449e7277 blockerbugs/cli.py 694736d098b6285c53db91f91350360d478c4229 .gitignore c5f8d92163a31685a35109eb4847315d9ebcd220
Diff: http://reviewboard-tflink.rhcloud.com/r/36/diff/
Testing -------
Wrote test suites. I've tested on my develop instance.
Thanks,
Ilgiz Islamgulov
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/36/ -----------------------------------------------------------
(Updated Aug. 9, 2013, 8:30 p.m.)
Review request for blockerbugs.
Changes -------
remove partial sync add error handling
Repository: blockerbugs
Description -------
Add partial updates sync. Display proposed/accepted fe/blocker bugs list associated with update on update list web page.
Diffs (updated) -----
testing/testfunc_update_sync.py 69f6bf8a459c71a9325cd633e7e33ce29354d7cd testing/test_updatesync_extract_information.py 1fc701ef79a74e27cf8223b56a04d63025b6509f testing/test_controllers.py f351832ad4350fcff3d67c0137ff62892639e908 blockerbugs/util/update_sync.py d664839ec1c5979dce980e7baad58154f4622e11 blockerbugs/util/bug_sync.py 49cce49740cd6f5b1f430f58c8d1b522e1f0b7e3 blockerbugs/templates/update_list.html a83dd9a422e51ea58e04ae58acd8fefe6eb1f79f blockerbugs/models/update.py 2ee3ca7ee62bd55bbd71555c13fecfab504c4f07 blockerbugs/controllers/main.py e00593a6835d8d679ee155d7da02c452ce4827d1 blockerbugs/cli.py 694736d098b6285c53db91f91350360d478c4229 .gitignore c5f8d92163a31685a35109eb4847315d9ebcd220
Diff: http://reviewboard-tflink.rhcloud.com/r/36/diff/
Testing -------
Wrote test suites. I've tested on my develop instance.
Thanks,
Ilgiz Islamgulov
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/36/ -----------------------------------------------------------
(Updated Aug. 16, 2013, 1:58 p.m.)
Review request for blockerbugs.
Changes -------
remove odd argument in sync updates method
Repository: blockerbugs
Description -------
Add partial updates sync. Display proposed/accepted fe/blocker bugs list associated with update on update list web page.
Diffs (updated) -----
testing/testfunc_update_sync.py 69f6bf8a459c71a9325cd633e7e33ce29354d7cd testing/test_updatesync_extract_information.py 1fc701ef79a74e27cf8223b56a04d63025b6509f testing/test_controllers.py f351832ad4350fcff3d67c0137ff62892639e908 blockerbugs/util/update_sync.py d664839ec1c5979dce980e7baad58154f4622e11 blockerbugs/util/bug_sync.py 49cce49740cd6f5b1f430f58c8d1b522e1f0b7e3 blockerbugs/templates/update_list.html a83dd9a422e51ea58e04ae58acd8fefe6eb1f79f blockerbugs/models/update.py 2ee3ca7ee62bd55bbd71555c13fecfab504c4f07 blockerbugs/controllers/main.py e00593a6835d8d679ee155d7da02c452ce4827d1 blockerbugs/cli.py 694736d098b6285c53db91f91350360d478c4229 .gitignore c5f8d92163a31685a35109eb4847315d9ebcd220
Diff: http://reviewboard-tflink.rhcloud.com/r/36/diff/
Testing -------
Wrote test suites. I've tested on my develop instance.
Thanks,
Ilgiz Islamgulov
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/36/#review69 -----------------------------------------------------------
Ship it!
Looks good to me - it's now working in my dev env with that last change.
For future code, can you not make arbitrary stylistic changes to logic unless you're in that code? I'm not going to say "back the changes out" this time but it adds churn when it's not really needed.
- Tim Flink
On Aug. 16, 2013, 1:58 p.m., Ilgiz Islamgulov wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/36/
(Updated Aug. 16, 2013, 1:58 p.m.)
Review request for blockerbugs.
Repository: blockerbugs
Description
Add partial updates sync. Display proposed/accepted fe/blocker bugs list associated with update on update list web page.
Diffs
testing/testfunc_update_sync.py 69f6bf8a459c71a9325cd633e7e33ce29354d7cd testing/test_updatesync_extract_information.py 1fc701ef79a74e27cf8223b56a04d63025b6509f testing/test_controllers.py f351832ad4350fcff3d67c0137ff62892639e908 blockerbugs/util/update_sync.py d664839ec1c5979dce980e7baad58154f4622e11 blockerbugs/util/bug_sync.py 49cce49740cd6f5b1f430f58c8d1b522e1f0b7e3 blockerbugs/templates/update_list.html a83dd9a422e51ea58e04ae58acd8fefe6eb1f79f blockerbugs/models/update.py 2ee3ca7ee62bd55bbd71555c13fecfab504c4f07 blockerbugs/controllers/main.py e00593a6835d8d679ee155d7da02c452ce4827d1 blockerbugs/cli.py 694736d098b6285c53db91f91350360d478c4229 .gitignore c5f8d92163a31685a35109eb4847315d9ebcd220
Diff: http://reviewboard-tflink.rhcloud.com/r/36/diff/
Testing
Wrote test suites. I've tested on my develop instance.
Thanks,
Ilgiz Islamgulov
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/36/ -----------------------------------------------------------
(Updated Aug. 16, 2013, 3:07 p.m.)
Status ------
This change has been marked as submitted.
Review request for blockerbugs.
Repository: blockerbugs
Description -------
Add partial updates sync. Display proposed/accepted fe/blocker bugs list associated with update on update list web page.
Diffs -----
testing/testfunc_update_sync.py 69f6bf8a459c71a9325cd633e7e33ce29354d7cd testing/test_updatesync_extract_information.py 1fc701ef79a74e27cf8223b56a04d63025b6509f testing/test_controllers.py f351832ad4350fcff3d67c0137ff62892639e908 blockerbugs/util/update_sync.py d664839ec1c5979dce980e7baad58154f4622e11 blockerbugs/util/bug_sync.py 49cce49740cd6f5b1f430f58c8d1b522e1f0b7e3 blockerbugs/templates/update_list.html a83dd9a422e51ea58e04ae58acd8fefe6eb1f79f blockerbugs/models/update.py 2ee3ca7ee62bd55bbd71555c13fecfab504c4f07 blockerbugs/controllers/main.py e00593a6835d8d679ee155d7da02c452ce4827d1 blockerbugs/cli.py 694736d098b6285c53db91f91350360d478c4229 .gitignore c5f8d92163a31685a35109eb4847315d9ebcd220
Diff: http://reviewboard-tflink.rhcloud.com/r/36/diff/
Testing -------
Wrote test suites. I've tested on my develop instance.
Thanks,
Ilgiz Islamgulov
qa-devel@lists.fedoraproject.org