Hi Tamar,
I wanted to try reviewing a patch and this seemed simple enough so I gave it a
shot. Hopefully this saves some time of the maintainer that eventually
approves this :).
I don't see any bug in the code. I also tried running it on my own input and
the output was correct. So functionally the patch is good. I have some
remarks about the style though:
- You sometimes put a space between function name and parentheses. This
doesn't look python-ish and isn't consistent in the file.
- There's one very long line (check_GNU_style_lib.py:335). I would shorten it
so it is at most 79 characters long.
- On the same line, there is a space after { but not before }. For
consistency, I would erase the space after {
- On the same line there are spaces after :. I think a more python-ish way
would be not to have those spaces there. Here I'm maybe being too pedantic
so feel free to ignore this. I think it will look nice either way.
To summarize the last 3 points, I would replace this
errlines.append({ "file" : locs[0], "row" : locs[1], "column" :
locs[2], "err" : e.console_error})
with this
errlines.append({"file": locs[0], "row": locs[1],
"column": locs[2], "err": e.console_error})
Cheers,
Filip Kastl
