Merge lp://qastaging/~vicamo/upstart/xenial-escape-systemd-strings into lp://qastaging/~ubuntu-core-dev/upstart/xenial
Proposed by
You-Sheng Yang
| Status: | Merged | ||||
|---|---|---|---|---|---|
| Merged at revision: | 1632 | ||||
| Proposed branch: | lp://qastaging/~vicamo/upstart/xenial-escape-systemd-strings | ||||
| Merge into: | lp://qastaging/~ubuntu-core-dev/upstart/xenial | ||||
| Diff against target: |
83 lines (+51/-4) 1 file modified
extra/upstart-local-bridge.c (+51/-4) |
||||
| To merge this branch: | bzr merge lp://qastaging/~vicamo/upstart/xenial-escape-systemd-strings | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Łukasz Zemczak | not an upstart maintainer | Approve | |
|
Review via email:
|
|||
Description of the change
upstart-
Android property names and values may contain invalid characters for systemd and breaks the mechanism here.
To post a comment you must log in.
Revision history for this message
| Łukasz Zemczak (sil2100) wrote | # |
review:
Approve
(not an upstart maintainer)
Revision history for this message
| Łukasz Zemczak (sil2100) | # |
Revision history for this message
| Dimitri John Ledkov (xnox) wrote | # |
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
In overall the proposed change looks fine an seems to work reasonably well. The same could have been realized without the new hexchar() function, but having it makes things potentially slightly faster and simplified.
That being said, I do not know the upstart code-base that well, so maybe it would be nice if some other maintainer took a quick look as well.
One stylistic recommendation: in the inline comments, the if, else if and else parts could use { } even in the single-line conditional commands. It's much cleaner this way and follows the upstart code style better.