Code review comment for ~paelzer/ubuntu/+source/chrony:merge-cosmic-3.3-1

Revision history for this message
Robie Basak (racb) wrote :

Looks good!

One question from an upload perspective:

2d3ebbe: is a policy-rc.d interaction required? I'm not sure so I'll defer to you or someone like xnox.

Minor typo, while you're fixing typos in README.container described inline.

From a workflow perspective:

9eec4ae->43b8bf9 seems to be transferred correctly but I can't find how it is or ever has been documented in debian/changelog. Should it perhaps now be squashed into cb49079? If yes, then I wouldn't expect it to be documented separately in the changelog for this upload (since it was part of the logical delta previously), but I would expect it to have been documented when the change was made in the past, unless it was done at the same time as the introduction of the original change to d/links.

Commit message for d4ebf92 appears inaccurate now, but the changelog message is correct. The upload is correct, but from a workflow POV it should be fixed by rebasing this branch to avoid carrying the inaccuracy forwards.

IMHO, instead of:

Forwarded: no (backport)
Origin: upstream, ...

The correct form in dep3 is:

Origin: backport, ...

"The field is really required only if the patch is vendor specific..." so I don't think a "Forwarded" entry is needed at all in the case of a backport.

review: Needs Information

« Back to merge proposal