Code review comment for lp:~al-maisan/launchpad/psds-model-changes-399186

Revision history for this message
Muharem Hrnjadovic (al-maisan) wrote :

Brad Crittenden wrote:
> Review: Approve code
> Hi Muharem,
>
> Thanks for the branch. It looks good except for the following small items:
>
> * Line 46 - End of line comments are discouraged by our coding standards. Please move to the previous line.
>
> * I think "The person who owns the package set at hand." would read better for most users as "The person who owns this package set."
>
> * Lines 91,110 - Capitalize Ubuntu.
>
> * Line 383 - typo 'dupliacte'

Hello Brad,

thank you very much for the review! I have revised the branch to
accommodate your suggestions.

Best regards

--
Muharem Hrnjadovic <email address hidden>
Public key id : B2BBFCFC
Key fingerprint : A5A3 CC67 2B87 D641 103F 5602 219F 6B60 B2BB FCFC

1=== modified file 'lib/lp/soyuz/interfaces/packageset.py'
2--- lib/lp/soyuz/interfaces/packageset.py 2009-10-27 15:42:04 +0000
3+++ lib/lp/soyuz/interfaces/packageset.py 2009-10-27 22:26:47 +0000
4@@ -35,13 +35,15 @@
5
6 class NoSuchPackageSet(NameLookupFailed):
7 """Raised when we try to look up an PackageSet that doesn't exist."""
8- webservice_error(400) #Bad request.
9+ # Bad request.
10+ webservice_error(400)
11 _message_prefix = "No such packageset"
12
13
14 class DuplicatePackagesetName(Exception):
15 """Raised for packagesets with the same name and distroseries."""
16- webservice_error(400) # Bad request.
17+ # Bad request.
18+ webservice_error(400)
19
20
21 class IPackagesetViewOnly(IHasOwner):
22@@ -56,7 +58,7 @@
23
24 owner = exported(Reference(
25 IPerson, title=_("Person"), required=True, readonly=True,
26- description=_("The person who owns the package set at hand.")))
27+ description=_("The person who owns this package set.")))
28
29 name = exported(TextLine(
30 title=_('Valid package set name'),
31@@ -336,7 +338,7 @@
32 title=_('Package set description'), required=True),
33 owner=Reference(
34 IPerson, title=_("Person"), required=True, readonly=True,
35- description=_("The person who owns the package set at hand.")),
36+ description=_("The person who owns this package set.")),
37 distroseries=Reference(
38 IDistroSeries, title=_("Distroseries"), required=False,
39 readonly=True, description=_(
40@@ -350,7 +352,7 @@
41 :param description: the description for the package set to be created.
42 :param owner: the owner of the package set to be created.
43 :param distroseries: the distroseries to which the new packageset
44- is related. Defaults to the current ubuntu series.
45+ is related. Defaults to the current Ubuntu series.
46 :param related_set: the newly created package set is to be related to
47 `related_set` (by being placed in the same package group).
48
49@@ -368,7 +370,7 @@
50
51 :param name: the name of the package set sought.
52 :param distroseries: the distroseries to which the new packageset
53- is related. Defaults to the current ubuntu series.
54+ is related. Defaults to the current Ubuntu series.
55
56 :return: An `IPackageset` instance or None.
57 """
58
59=== modified file 'lib/lp/soyuz/tests/test_packageset.py'
60--- lib/lp/soyuz/tests/test_packageset.py 2009-10-27 15:51:28 +0000
61+++ lib/lp/soyuz/tests/test_packageset.py 2009-10-27 22:21:31 +0000
62@@ -70,7 +70,7 @@
63
64 self.failUnlessRaises(
65 DuplicatePackagesetName, self.packageset_set.new,
66- u'kernel', u'A packageset with a dupliacte name', self.person1,
67+ u'kernel', u'A packageset with a duplicate name', self.person1,
68 distroseries=self.distroseries_experimental)
69
70 def test_new_duplicate_name_for_different_distroseries(self):

« Back to merge proposal