[Aptitude-devel] aptitude-qt VCS usage and code review

[Piotr Galiszewski]

2010/7/10 Daniel Burrows <dburrows at debian.org>:
> On Sat, Jul 10, 2010 at 12:24:41AM +0200, Piotr Galiszewski <piotr at galiszewski.pl> was heard to say:
>> 2010/7/9 Daniel Burrows <dburrows at google.com>:
>> >   Item (2) confuses me -- what is the name for, exactly?  If it's just the
>> > name the user gave, why is it immutable?  I'm going to assume that
>> > was an oversight.
>>
>> Name  is shown in filters_list on the right side of packages tab. It
>> is immutable, because I have forgotten to add setter. I planned to do
>> this when creating manage filters window. As  name is confusing
>> probably display or display_name would be better.
>
>  Ok.  The user can edit that, I assume?
>

Yes.

>> >   Item (3) can be handled at the same time as item (2), by creating
>> > a separate "holder" class that represents the user's definition of the
>> > filter and is modified when the definition is modified.  This has the
>> > extra advantage of letting you switch to any other filter, not just one
>> > implemented by the same concrete class.
>> >   i.e., you would have "package_filter", which is an interface for
>> > predicates
>> > on packages, and "package_filter_definition", which contains a name and a
>> > package_filter (pointer), both of which have mutators, and which has two
>> > signals corresponding to those fields, emitted when they change.
>> >   Daniel
>>
>> If I understand correctly package_filter contains
>> accept_package(package) function and for example lists of accepted
>> tags/sections or other mutators. Later every package_filter_definition
>> sets proper values of this mutators? I have read it several times and
>> it is still not entirely clear for me. Some sample code will be really
>> nice :)
>
>  package_filter is a pure virtual interface with "bool package_matches()"
> defined somewhere in it.  package_filter objects are immutable.
>
>  package_filter_definition contains a package_filter and a name.  The
> package_filter can be set to a different package_filter and the name
> can be set to a different name.  Setting either of these fields triggers
> a signal, so interested parties can notice when the filter has changed.
>
>  Actually, I'm not even 100% sure that package_filter_definition needs
> to be mutable; I just assumed there was a good reason for that. :)
>

Thanks for clarification.

>
>  I got partway through your second patch, but I think I need to break
> for the night now.  Sorry.
>

No problem. I added some documentation to second patch. status_widget
is missing, but unfortunately I have to go now, and I will be
available on tomorrow's morning. Sorry about that!

Is this documentation acceptable? I do not have big experience with
writing docs so please be tolerant. Also the grammar and spelling
probably has to be fixed.

I hope that tomorrow will be much more productive from my side

>  Daniel
>
-- 
Regards
Piotr Galiszewski