← Back to team overview

ulc-devs team mailing list archive

Re: code merging request

 

Hi Frederik,

On Sat, Dec 10, 2011 at 3:29 PM, Frederik Elwert
<frederik.elwert@xxxxxx>wrote:
>
> Two remaining remarks:
>
>      * There was some trailing whitespace in the code. This does not
>        generally hurt, but is considered ugly. It is difficult to see,
>        so this happens quite often. Two ways to deal with it: The gedit
>        developer plugins report trailing whitespace, and there is an
>        gedit tool (in the "external tools" section) to remove trailing
>        whitespace. I removed it, but you could try to consider this in
>        future commits.
>

ok, I didn't think of this. I now tried the developer plugins for gedit -
the "check syntax and style" option is really helpful!

     * When you address multiple issues in one commit, it is best to
>        have a one-line summary in the commit message, then leave a
>        blank line, and then have a list with details. Often, only the
>        first line of a commit message is shown, and in these cases, it
>        makes more sense to have a summary than the first item of a
>        list.
>

 thanks for the hint, I did not know this ;)


> I just noticed two glitches in the current implementation: Firstly,
> whitespace was also listed in the "numbers" category. I fixed this by
> checking explicitly for "char.is_digit()". Secondly, it seems that upper
> and lower results are switched: lower-case chars appear in the "upper"
> category, and vice versa. I did not find out why, so I left it for now.
>

I was aware of the number category displaying everything that wasn't a
lower or upper case letter. I actually forgot to fix this, so thank you. I
also remember that I had problems with the two categories (upper, lower)
displaying wrong results, but I thought I had fixed it already. I
overlooked the code again but I couldn't find anything. To verify, I
downloaded the branch again (unity-lens-contacts) and after installing it
seems to display the results correctly! Maybe try to apt-get remove --purge
unity-lens-contacts and then rebranching and reinstalling the code...

regards,
-- 
Patrick Seemann

launchpad: ~patrickseemann
irc.freenode.net: patsee14

Follow ups

References