Code review comment for lp:~songofacandy/bzr/i18n-command

Revision history for this message
methane (songofacandy) wrote :

> 8 +
>
> Spurious ?

I've removed it.

>
> 20 + i18n.uninstall()
> 21 +
>
> Hmm, that's weird, who did install what ? Why do we need to start
> uninstalling ?

I think of many other commands like log or status use i18n in the future.
I18n only breaks selftest.

>
> Said otherwise, this reveals some weird initialization and I
> think we want a better way to achieve it.
>
> To avoid a controversial discussion about general deployment I
> think you should focus on initializing the i18n/i10n only for the
> help command. We can revisit that later once we are happy with
> the results.

I am not against on enabling i18n only for help command until other
commands needs i18n.

But, how? When selftest calls help, enabling i18n breaks many tests.
I don't think replacing `i18n.uninstall()` with global variable like
`opt_no_i18n=True` is beautiful answer.

One idea come to my mind is adding `use_i18n = False` class attribute
to the Command class. The cmd_help overrides it with `use_i18n = True`.
Then, selftest overrides `use_i18n = False` again.

Do you think OK with this idea?

« Back to merge proposal