Bug 1030972 - Import tasks from Bugzilla #170
Conversation
| @@ -0,0 +1,54 @@ | |||
| # This Source Code Form is subject to the terms of the Mozilla Public | |||
There was a problem hiding this comment.
I would call this bugzilla_utils just to make the name a bit clearer.
|
Thanks for all the comments so far @bobsilverberg. I assume there are more coming, and I'm keeping an eye on the Bugzilla discussion as well. |
|
@mjzffr: Yes, I am still in the process of reviewing it. I have also made some comments on the related bug which would be worthwhile reading and commenting on. They have to do with the UI. I will make a comment when I am done with this review so you'll know. Thanks for all the hard work you obviously put into this. Although I have a lot of comments this is an excellent pull request. |
| TaskInvalidationCriterion, | ||
| form=TaskInvalidationCriterionForm) | ||
|
|
||
| class TaskImportBatchForm(forms.ModelForm): |
There was a problem hiding this comment.
Missing a blank line before this. Please run a pep8 checker against the code which will alert you to these types of issues.
mjzffr
force-pushed
the
bugzillataskimport
branch
from
fa10901
to
887c91c
Compare
Conflicts: oneanddone/tasks/templates/tasks/form.html
Conflicts: oneanddone/tasks/templates/tasks/form.html
Conflicts: oneanddone/tasks/templates/tasks/form.html
Conflicts: oneanddone/tasks/templates/tasks/form.html
mjzffr
force-pushed
the
bugzillataskimport
branch
3 times, most recently
from
e585869
to
615c0bf
Compare
mjzffr
force-pushed
the
bugzillataskimport
branch
from
615c0bf
to
b8e400c
Compare
|
Thanks for all your hard work on this @mjzffr. It looks great and works well! There are still a few tiny issues, but I understand that you will not have any more time to work on this between now and Tuesday, when we aim to implement it, so tomorrow (Monday) morning I'm going to squash your commits and merge this so you can get the credit for all of this work. Then I'll add a separate commit for my small changes. We can also add the unit tests in a separate commit later (those are still coming, right?). |
mjzffr
force-pushed
the
bugzillataskimport
branch
from
b8e400c
to
eecffb8
Compare
|
@bobsilverberg Thanks! The tests are coming, I promise. I'm learning how and when to use Mock and patch as I go along (and how to fake requests), so it's slow, but it's going. |
|
I feel like we should make a t-shirt to commemorate this pull request |
|
@Osmose you just earned a commemorative plaque by posting the 100th comment. =D I added some tests so y'all can see where I'm headed and maybe steer me in the right direction in terms of using Mock and such. I was in the middle of writing tests to see if the right template is used depending on which "stage" is posted. I think I need to pass in complete form data to get those tests to work. |
|
Everything in this PR, except for the last commit which just includes some tests, landed in d9374cc, so I'm closing it (yay!). @mjzffr Please open a new PR with just the tests. I have at least one comment on the tests you've written thus far, but I might not get to that for a few days as I'm trying to finish up this release. Thanks again for all of your hard work on this! |
@bobsilverberg @Osmose r?