Code review comment for lp:~michael.nelson/launchpad/distro-series-difference-basic-model

Revision history for this message
Abel Deuring (adeuring) wrote :

Hi Michael,

nice work! I have just two nitpicks:

> === added file 'lib/lp/registry/interfaces/distroseriesdifference.py'
> --- lib/lp/registry/interfaces/distroseriesdifference.py 1970-01-01 00:00:00 +0000
> +++ lib/lp/registry/interfaces/distroseriesdifference.py 2010-08-27 09:31:04 +0000

[...]

> + activity_log = Text(
> + title=_('A log of activity and comments for this difference'),
> + required=False, readonly=True)

s/difference/difference./

> === added file 'lib/lp/registry/tests/test_distroseriesdifference.py'
> --- lib/lp/registry/tests/test_distroseriesdifference.py 1970-01-01 00:00:00 +0000
> +++ lib/lp/registry/tests/test_distroseriesdifference.py 2010-08-27 09:31:04 +0000

[...]

> + def test_new_non_derived_series(self):
> + # A DistroSeriesDifference cannot be created with a non-derived
> + # series.
> + distro_series = self.factory.makeDistroSeries()
> + self.assertRaises(
> + NotADerivedSeriesError,
> + self.factory.makeDistroSeriesDifference,
> + derived_series=distro_series)

I would prefer something like

    distroseriesdifferencesource_factory = getUtility(IDistroSeriesDifferenceSource)
    self.assertRaises(
        NotADerivedSeriesError, distroseriesdifferencesource_factory.new, ...)

This makes it a bit more explicit which method is tested.

review: Approve (code)

« Back to merge proposal