Code review comment for lp:~jelmer/bzr/result-mentions-tags

Revision history for this message
Vincent Ladeuil (vila) wrote :

[needsinfo] It seems merge_to changes caused problems in the past
(see bzrlib.tags._merge_tags_if_possible), what's your feeling on
changing the signature again and the possible fallouts ? (loom ?)

Or did I misread the diff and merge_to didn't return anything
before ? Or does it matter nevertheless ?

[needsfixing] There is a lot of duplication between
PullResult.report() and BranchPushResult.report()

But one is using to_file_write.write() and the other
trace.note(). This doesn't feel right, shouldn't they both
provide the same feedback to the user ?

[PEP8 nit]
75 + result.tag_updates, result.tag_conflicts = self.source.tags.merge_to(
76 + self.target.tags, overwrite, ignore_master=not merge_tags_to_master)

lines too long.

[2.4-worthy] Shouldn't that be backported for the benefit of the
package importer ?

review: Needs Fixing

« Back to merge proposal