Merge lp://staging/~danilo/pygettextpo/simplify into lp://staging/pygettextpo
| Status: | Needs review |
|---|---|
| Proposed branch: | lp://staging/~danilo/pygettextpo/simplify |
| Merge into: | lp://staging/pygettextpo |
| Prerequisite: | lp://staging/~danilo/pygettextpo/pep8-fixes |
| Diff against target: |
169 lines (+40/-92) 1 file modified
gettextpo.c (+40/-92) |
| To merge this branch: | bzr merge lp://staging/~danilo/pygettextpo/simplify |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Benji York (community) | code | Approve | |
|
Review via email:
|
|||
Description of the change
Simplify the C code behind simple set_msg* methods to facilitate code reuse.
Since I'd like to extend it to support all the other current features of libgettextpo, and a bunch of other methods would need the same code for executing a bunch of actions (set_msgctxt, set_extracted_
Alternative approach for this would have been to define a macro, which might be faster (no passing around of function pointers), but it would have resulted in larger binaries and would have been uglier (\ continuation lines and such).
'make check' confirms no tests are broken (all the modified methods are tested in a basic way, but that should be sufficient for the type of changes here).
| Colin Watson (cjwatson) wrote | # |
Unmerged revisions
- 28. By Данило Шеган
-
Merge pep8-fixes.
- 27. By Данило Шеган
-
Remove "helper" from the helper function name: it confused me a bare month after I introduced it because I read it as "helper message" instead of a "helper function on message class".
- 26. By Данило Шеган
-
Reuse the new helper function in other similar methods.
- 25. By Данило Шеган
-
Extract po message setter pattern into a helper method.
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_
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.