Code review comment for lp:~spiv/bzr-builder/merge-subdirs-479705

Revision history for this message
James Westby (james-w) wrote :

Hi,

Thanks for working on this.

I have some concerns about the utility of it. If the branch
you are merging doesn't make any modifications outside of
./debian/ then this makes no difference, therefore it must
be for the case where it does.

Is this then to take the packaging without any modifications
to the source, so that it is a vanilla packaging?

If so, then it is just assumed that this won't be done if
some of the modifications are needed, or if the packaging
depends on some of the modifications?

That would be acceptable, but I wonder why we want to encourage
people to do things this way, instead of say, layering another
branch on top that undoes the unwanted things? Obviously the two
are not equivalent, but the latter seems to fit better with the
model to me.

As for a code review, two things mean that this is not mergeable
as is:

  * Needs a format bump.
  * Needs an update to the documentation so that people know it is possible.

Other than that the changes look fine with one obvious exception:

45 + :param subpath: XXX

Thanks,

James

review: Needs Resubmitting

« Back to merge proposal