|
Hi, I sent this to the list a week ago. Is there still some feedback coming? Regards Adi -------- Original Message --------
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 >> git://github.com/adsworth/kupfer.git >> 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. Adi |