[Aptitude-devel] Review of 376324

[Piotr Galiszewski]

2010/8/3 Daniel Burrows <dburrows at debian.org>:
>  Just some minor nits.  Also, this doesn't seem to be dependent on its
> parents.  Is it OK if I cherry-pick it onto master?  (you'll need to
> rewrite its branch to drop this patch, then)  I did a quick test on my
> machine, and this patch seems to be perfectly happy sitting on top of
> current master instead of on 002.1-packages_tab.
>

I have no problems with this ;) For testing purposes I am creating
code snapshots, as Arthur suggested me on DebConf

[snip]
>> +     /** \brief Register a slot to be invoked when the defined filter changes. */
>> +     sigc::connection connect_filter_changed(const sigc::slot<void> &slot);
>> +
>> +     sigc::signal1<void, std::string> name_changed_signal;
>> +     sigc::signal0<void> filter_changed_signal;
>
>  Would it make sense to pass along the filter with this signal, for
> consistency?
>

It is not necessary, as this signal is only used for invalidating
model filtering. But I can do this

[snip]

>> +      /** \brief Create a package_filter_definition.
>> +       *
>> +       *  \param name   The name of the new filter definition.
>> +       *  \param filter The filter defined by the new filter definition.
>> +       */
>> +      package_filter_definition_ptr
>> +       make_package_filter_definition(const std::string &name,
>> +                                      const package_filter_ptr &filter);
>> +    }
>
>  Per our recent discussion, maybe this should be a static ::create() method?
>

Yes. I have to have a look on all previous patches and fix this issue.
Thanks for all comments!

>  Daniel
>
> _______________________________________________
> Aptitude-devel mailing list
> Aptitude-devel at lists.alioth.debian.org
> http://lists.alioth.debian.org/mailman/listinfo/aptitude-devel
>
-- 
Regards
Piotr Galiszewski