Hi Clay.

I checked the differential webrev. Thanks for making this change.

Only a couple of small nits:

publish-manifest.py: comment on 188-189 refers to None/none but should 
refer to "unbounded". (Thanks for adding comments at 191 and 195!)

881: "Criteria" is plural of "criterion", so with proper grammar this 
would be: "... at least one criterion needed!"

Thanks,
Jack

On 10/23/09 15:45, Clay Baenziger wrote:
> Hi Jack,
> You request brings up a very good point, I think findCollidingCriteria 
> should be ignorant of what type of manifest it's called with. I've 
> moved the check to the program main-line.
>
> if __name__ == '__main__':
> gettext.install("ai", "/usr/lib/locale")
> data = DataFiles()
>
> # check that we are root
> if os.geteuid() != 0:
> raise SystemExit(_("Error:\tNeed root privileges to run"))
>
> # load in all the options and file data
> parseOptions(data)
>
> # if we have a default manifest do default manifest handling
> if data.manifestName() == "default.xml":
> doDefault(data)
>
> # if we have a non-default manifest first ensure it is a unique criteria
> # set and then, if unique, add the manifest to the criteria database
> else:
> # if we have a None criteria from findCriteria then the manifest has
> # no criteria which is illegal for a non-default manifest
> if data.findCriteria() is None:
> raise SystemExit(_("Error:\tNo criteria found " +
> "in non-default manifest -- "
> "at least one criteria needed!"))
> findCollidingManifests(data, findCollidingCriteria(data))
> insertSQL(data)
>
> # move the manifest into place
> placeManifest(data)
>
> Or see http://cr.opensolaris.org/~clayb/publish_manifest/webrev2 for a 
> differential webrev.
>
> Thank you,
> Clay
>
> On Fri, 23 Oct 2009, Jack Schwartz wrote:
>
>> Hi Clay.
>>
>> Please add a comment in findCollidingCriteria that it is called only 
>> for non-default manifests.
>>
>> Otherwise the code looks great.
>>
>> Thanks,
>> Jack
>>
>>
>> On 10/22/09 17:27, Clay Baenziger wrote:
>>> Oops, I'm forgetting my own e-mail asking for the PyLint output as 
>>> well, which follows:
>>>
>>> Raw metrics
>>> -----------
>>>
>>> +----------+-------+------+---------+-----------+
>>> |type |number |% |previous |difference |
>>> +==========+=======+======+=========+===========+
>>> |code |503 |59.25 |504 |-1.00 |
>>> +----------+-------+------+---------+-----------+
>>> |docstring |138 |16.25 |138 |= |
>>> +----------+-------+------+---------+-----------+
>>> |comment |110 |12.96 |110 |= |
>>> +----------+-------+------+---------+-----------+
>>> |empty |98 |11.54 |98 |= |
>>> +----------+-------+------+---------+-----------+
>>>
>>> Messages
>>> --------
>>>
>>> +-----------+-----------+
>>> |message id |occurences |
>>> +===========+===========+
>>> |C0103 |74 |
>>> +-----------+-----------+
>>> |C0301 |15 |
>>> +-----------+-----------+
>>> |W0212 |9 |
>>> +-----------+-----------+
>>> |R0912 |5 |
>>> +-----------+-----------+
>>> |F0401 |4 |
>>> +-----------+-----------+
>>> |W0611 |2 |
>>> +-----------+-----------+
>>> |W0621 |1 |
>>> +-----------+-----------+
>>> |W0311 |1 |
>>> +-----------+-----------+
>>> |R0915 |1 |
>>> +-----------+-----------+
>>> |R0911 |1 |
>>> +-----------+-----------+
>>> |R0902 |1 |
>>> +-----------+-----------+
>>> |C0324 |1 |
>>> +-----------+-----------+
>>> |C0302 |1 |
>>> +-----------+-----------+
>>>
>>> Global evaluation
>>> -----------------
>>> Your code has been rated at 7.12/10 (previous run: 7.10/10)
>>> If you commit now, people should not be making nasty comments about 
>>> you on c.l.py
>>>
>>> Thank you,
>>> Clay
>>>
>>> On Thu, 22 Oct 2009, Clay Baenziger wrote:
>>>
>>>> Hi all,
>>>> If anyone has a few moments to review some PEP8 changes and a 20 
>>>> line bug fix, I'd love to push some code before folks start pushing 
>>>> for our Python 2.6 blitz. I've fixed the following UI bugs in 
>>>> publish-manifest and a typo in the default A/I manifest:
>>>>
>>>> 4318 - publish-manifest needs to error about no criteria in 
>>>> non-default
>>>> manifests
>>>> 4326 - Ugly error if wrong data provided in criteria manifest
>>>> 11984 - Typo in suggestion in AI's default.xml manifest
>>>>
>>>> To show how differential webrevs can work for PEP8 changes see the 
>>>> break out of webrevs below:
>>>>
>>>> Full Webrev (big):
>>>> http://cr.opensolaris.org/~clayb/publish_manifest
>>>>
>>>> PEP8 Changes:
>>>> http://cr.opensolaris.org/~clayb/publish_manifest/pep8
>>>>
>>>> Bug Fixes (only 21 lines!):
>>>> http://cr.opensolaris.org/~clayb/publish_manifest/diff
>>>>
>>>> Bugs:
>>>> publish-manifest needs to error about no criteria in non-default 
>>>> manifests:
>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4318
>>>>
>>>> Ugly error if wrong data provided in criteria manifest
>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4326
>>>>
>>>> Typo in suggestion in AI's default.xml manifest
>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=11984
>>>>
>>>>
>>>>
>>> _______________________________________________
>>> caiman-discuss mailing list
>>> caiman-discuss at opensolaris.org
>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>
>>


Reply via email to