ulc-devs team mailing list archive

[Frederik Elwert <frederik.elwert@xxxxxx>]

Hi Patrick,

I did now find the time to look over your code. I think you addressed
almost all issues, so I merged it into trunk.

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.
      * 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.

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.

This is not really important, as we will overwrite this with real code
anyway. But if we want to spread this as an example for a basic python
lens and maybe turn it into a quickly template, it would be nice to have
it working smoothly. Nothing to spent much time with, but if you find
the issue by chance, we could fix it.

Thanks again for the work!

Regards
Frederik