Code review comment for lp:~doxxx/bzr/mergetools

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

Almost there (finally ;) !

36 + def get_default_merge_tool(self):
37 + return self.get_user_option('bzr.default_mergetool')
38 +

No need for that since there is no added check or feature there (and it shouldn't).

146 +
147 + def _get_command_line(self):
148 + return self._command_line
149 +
150 + def _set_command_line(self, command_line):
151 + self._command_line = command_line
152 + self._cmd_list = cmdline.split(self.command_line)
153 +
154 + command_line = property(_get_command_line, _set_command_line)

This isn't used in your proposal so it's not needed, if you need it for another mp, let's review it there please.

115 +from bzrlib import (
116 + cmdline,
117 + config,
118 + errors,
119 + osutils,
120 + trace,
121 + ui,
122 + workingtree,

Only cmdline and osutils seem to be necessary there.

Make sure you target 2.4 not 2.3 anymore (so move your news entries in the right files).

review: Needs Fixing

« Back to merge proposal