Code review comment for lp://staging/~danilo/pygettextpo/simplify

Revision history for this message
Benji York (benji) wrote :

This branch looks good. I just have a couple of cosmetic things to
mention.

Is the space between "*" and "setter" intentional in the below (lines
11-12 of the diff):

    message_set_field(PyPoMessage *self, PyObject *args, const char *field,
                            po_value_setter_t * setter)

Also, is it just me or is the indentation of the second line above (line
12 of the diff) odd? I don't know the prevailing style in the
surrounding code, but I would expect an indentation of 4 spaces, 8
spaces, or aligning the start of the second line with the open
parenthesis in line 11.

Again, I'm not familiar with the style guides of pygettextpo, but most C
code bases try to avoid non-braced if blocks like those on line 18, 21,
and 26.

Oh, I see now that the code was mostly just moved, so perhaps the
one-line-bodies are OK.

review: Approve (code)

« Back to merge proposal