Re: GMail plugin extension

Hi Ulrik,

On 08.09.2010 13:17, Ulrik Sverdrup wrote:
2010/9/7 Adi J. Sieker<adi sieker info>:

I just pushed an update to the GMail plugin to
The plugin now downloads all data and the contact leaf now provides a
content source,
which contains most data from the contact. Currently that's all email
addresses as an EmailContact,
all phone numbers, postal addresses and IM account's as TextLeafs.

All feedback welcome

Hi Adi,

In general, it's a good idea. However as it is now, this change
conflicts with contacts grouping (since grouping uses the contents
"slot" to show the contacts grouped together); this patch does away
with grouping all together of course (no longer using EmailContact).
So there is no direct conflict, but it needs to be explicitly
addressed somehow. It doesn't mean that this can't be solved, or that
this information could not be exposed in a different way.

I tried to get my head around what a GroupingLeaf does, but I didn't find any documentation on that. Google just shows commit messages. So if I could get an introduction to GroupingLeaves or you have suggestion as to how I can rework it.

If you don't mind, the code review reads like this:

I did ask for it. :)

The commit log message must adhere to git standards
(the first line must not be longer than 60-70 letters ideally).
Since I pushed the commit already. I recreated my github fork, cloned, commited and pushed again. So if someone added me as a remote and fetched it would probably be best to delete the remote and add it again.

The patch also has some whitespace
problem (git diff  should highlight these, if you have git configured
for color, else use git diff --check);

Corrected the trailing whitespaces.

and get_items is better written
as a generator.
And reworked get_items.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]