Code review comment for lp:~vila/bzr/deprecation-warning-preference

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

>>>>> "jam" == John A Meinel <email address hidden> writes:

<snip/>

    jam> Well, *I* may not care about using --1.9, but I don't
    jam> want to be using --weave anymore. Not to say other
    jam> people are in that boat.

What I meant (and forgot to add when writing the cover letter) is
that it's highly likely that people are coherent and use the same
format inside project boundaries so giving a path-based control
over the warning should be pretty close in ease of use to a
format-based control.

But the later is harder to implement and likely harder to use
(and my main point was that we don't want people to get involved
into all of your existing formats if we can avoid it :-)

    jam> v- Is the extra 's' for extra suppression ?

Exactly, I think I made that typo at least 50 times so be
grateful that I left only one as I'm grateful you caught it :D

    jam> +* The ``suppresss_warnings`` configuration option has been introduced and
    jam> + accept the ``format_deprecation`` value to disable the corresponding
    jam> + warning for repositories. It can be set to in either ``bazaar.conf``,
    jam> + ``locations.conf`` or ``branch.conf``.
    jam> + (Ted Gould, Matthew Fuller, Vincent Ladeuil)

    jam> You can use

    jam> config.LocationConfig(repo.bzrdir.root_transport.base) if you wanted to
    jam> support per-repo configuration.

I almost implemented it. I almost mentioned it in the cover letter.

But I had a bad feeling about it, it smells like a workaround. I
don't want to create yet another config file but at the same time
I have a feeling that it may address some other needs.

This needs discussion anyway so I didn't feel bad enough to
implement config.LocationConfig(repo.bzrdir.root_transport.base).

    jam> Otherwise:

    jam> review: approve
    jam> merge: approve

Thanks, you didn't comment on the remaining points so I've filed
bug #497694, comments there welcome.

    Vincent

« Back to merge proposal