[pkg-discuss] Draft code review for a piece of @current

[Brock Pytlik]

Greetings all,

Here's the webrev:
https://cr.opensolaris.org/action/browse/pkg/bpytlik/7148059-draft

It's a draft of the first part of the changes needed for @current. These
roughly correspond to what's needed for step 2. I'd appreciate if people
could take a look at the changes to fmri.py and version.py since a major
change there might have significant impact on the rest of what I have
planned for @current.

Thanks,
Brock
_______________________________________________
pkg-discuss mailing list
pkg-d...@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

[Shawn Walker]

On 03/07/12 19:25, Brock Pytlik wrote:
> Greetings all,
>
> Here's the webrev:
> https://cr.opensolaris.org/action/browse/pkg/bpytlik/7148059-draft
>
> It's a draft of the first part of the changes needed for @current. These
> roughly correspond to what's needed for step 2. I'd appreciate if people
> could take a look at the changes to fmri.py and version.py since a major
> change there might have significant impact on the rest of what I have
> planned for @current.

If you could update the docstrings in pkg.version.CurrentVersion, that
would go a long way to helping understand the approach taken here.

It's very hard to discern why some things are done right now because the
comments don't match at all.

-Shawn

[Brock Pytlik]

On 03/08/12 09:34, Shawn Walker wrote:
> On 03/07/12 19:25, Brock Pytlik wrote:
>> Greetings all,
>>
>> Here's the webrev:
>> https://cr.opensolaris.org/action/browse/pkg/bpytlik/7148059-draft
>>
>> It's a draft of the first part of the changes needed for @current. These
>> roughly correspond to what's needed for step 2. I'd appreciate if people
>> could take a look at the changes to fmri.py and version.py since a major
>> change there might have significant impact on the rest of what I have
>> planned for @current.
>
> If you could update the docstrings in pkg.version.CurrentVersion, that
> would go a long way to helping understand the approach taken here.
>

I've changed two, and removed the rest. The webrev's updated in place.
There really isn't much going on in CurrentVersion's code since most of
the methods just raise NotImplemented. In anticipation of the next
request, I've also grouped all those together.

Please let me know where any remaining confusion comes from.

Thanks,
Brock