Merge lp://staging/~stevenk/launchpad/dsd-hide-child-diff into lp://staging/launchpad
Proposed by
Steve Kowalik
| Status: | Merged |
|---|---|
| Approved by: | Steve Kowalik |
| Approved revision: | no longer in the source branch. |
| Merged at revision: | 12713 |
| Proposed branch: | lp://staging/~stevenk/launchpad/dsd-hide-child-diff |
| Merge into: | lp://staging/launchpad |
| Diff against target: |
110 lines (+51/-8) 3 files modified
lib/lp/registry/browser/distroseriesdifference.py (+5/-0) lib/lp/registry/browser/tests/test_distroseriesdifference_views.py (+45/-7) lib/lp/registry/templates/distroseriesdifference-listing-extra.pt (+1/-1) |
| To merge this branch: | bzr merge lp://staging/~stevenk/launchpad/dsd-hide-child-diff |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Gavin Panella (community) | Approve | ||
|
Review via email:
|
|||
Commit message
[r=allenap][bug=746201] Hide the child difference if the child's source version is the same as the base version.
Description of the change
Don't display the child difference diff if the child's source version is the same as the base version, because the generated diff will be empty.
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
Nice :)
[1]
+ tags = soup.findAll(
+ self.assertEqual(2, len(tags))
This is a bit fragile because it's so non-specific.
Consider changing it to:
tags = soup.find('ul', 'package-
[2]
There are no unit tests for the view's display_child_diff
property. It's pretty simple and I think it's okay how you've done
it. I'm just wondering whether or not it would be better to test
display_child_diff in isolation (and the template in isolation, by
patching the view for example), and then having these integration
tests on top. That's more typing, would probably be more "pure", but I
wonder if it would buy us anything? I'm thinking out loud, I'm not
asking you to make any changes, just interested if you have an
opinion.