Code review comment for lp:~benji/launchpad/wadl-refactoring
Revision history for this message
| Benji York (benji) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
35a32c5
(Get the code!)
On Thu, Sep 23, 2010 at 12:34 PM, Gavin Panella
<email address hidden> wrote:
> Review: Needs Fixing code
> Nice clean up :)
>
> I have a few fairly minor comments. [4] needs fixing before this
> branch is safe to land.
>
>
> [1]
>
> + self.assert_
>
> I think this might be better as:
[snip]
Done.
> [2]
>
> + self.assert_('<html ' in html)
>
> Same as [1].
I had to implement a Contains matcher, but that wasn't hard.
> [3]
>
> Unless it's useless and/or noisy, I think stderr should be left alone,
Agreed.
> I also think the exit code should be checked, so:
Done.
> [4]
>
> -import _pythonpath
> -
>
> It's ugly, looks like lint, but you'll need this.
Fixed.
> It may be worth adding a test to show that the script can run
> successfully, even if it's only to show the help.
Since we can be sure that the script runs because it is run (almost)
every time make is run, I decided not to implement this.
--
Benji York