Code review comment for lp:~jelmer/bzr-builder/force-native

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

On Tue, 2010-12-07 at 16:56 +0000, James Westby wrote:
> Thanks for doing this, it is fairly unobtrusive and solves a common problem,
> so I'm happy to have it.
>
> 70 + f.write("3.0 (native)")
> Should that have a newline?
Hmm, that's a good point. Both with and without seems to be allowed, but
with is more common. Fixed.

> 75 +def force_native_format(working_tree_path):
> Should this die if it doesn't recognise the format marker?
Yeah, better safe than sorry (and we don't support "2.0" at this point).

> 93 + Option("force-native",
> 94 + help="Force the source package format to be native."),

> Should this be an option? I can't really think of a reason why people
> wouldn't want this. Is it just being conservative? Maybe have the option,
> but invert the default?
Yeah, I was mainly being conservative. But you're right, without this
option things wouldn't really work anyway. I've killed it.

Cheers,

Jelmer

« Back to merge proposal