Merge lp:~ted/bzr/deprecation-warning-preference into lp:bzr

Allows for users to have a local configuration option to suppress the format deprecation warning.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ted Gould wrote:
> Ted Gould has proposed merging lp:~ted/bzr/deprecation-warning-preference into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
>
> Allows for users to have a local configuration option to suppress the format deprecation warning.
>

I wouldn't call this 'deprecation-warning' unless it turned off all
other deprecation warnings.

What about "format-deprecation-warning"?

I'll also note that most of our settings use underscore rather than dash

post_commmit_mailer
launchpad_username
smtp_server
mail_client
bugtracker_XXX_url
etc

So

format_deprecation_warning

Seems a better term...

 review: needs_fixing

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAksUPpsACgkQJdeBCYSNAAOmMACfXaPkGZR/dMgJqVaWIwNrFfcG
Vx0An3S6NfYQnojccqn4+RvYOawQI5Ae
=thjQ
-----END PGP SIGNATURE-----

This needs to get added to the configuration help text. Other than that

 review: approve

On Mon, 2009-11-30 at 21:54 +0000, John A Meinel wrote:
> I wouldn't call this 'deprecation-warning' unless it turned off all
> other deprecation warnings.

Changed to "format_deprecation_warning" in r4842.

On Mon, 2009-11-30 at 22:15 +0000, Robert Collins wrote:
> This needs to get added to the configuration help text. Other than that

Updated in r4843

On further reflection, I'd like to change the setting name to skip_format_deprecation_warning or suppress_format_deprecation_warning. Otherwise, users set format_deprecation_warning is order not to get one which feels a little awkward.

Is that OK? If so, any preference?

2009/12/4 Ian Clatworthy <email address hidden>:
> Review: Needs Information
> On further reflection, I'd like to change the setting name to skip_format_deprecation_warning or suppress_format_deprecation_warning. Otherwise, users set format_deprecation_warning is order not to get one which feels a little awkward.

Sounds better to me. Or even

 suppress_warnings = format_deprecation, ....

--
Martin <http://launchpad.net/~mbp/>

On Fri, 2009-12-04 at 07:42 +0000, Ian Clatworthy wrote:
> Is that OK? If so, any preference?

No, no preference :)

I just want "make bazaar not annoying for people who work on projects
that use older formats" -- but that seemed too long ;)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ian Clatworthy wrote:
> Review: Needs Information
> On further reflection, I'd like to change the setting name to skip_format_deprecation_warning or suppress_format_deprecation_warning. Otherwise, users set format_deprecation_warning is order not to get one which feels a little awkward.
>
> Is that OK? If so, any preference?

I was thinking "disable_format_deprecation_warnings" as well, but I
thought of it in the car rather than while near a machine. :)

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAksZIzYACgkQJdeBCYSNAAP7ygCePZ/IH3qowR4lsqcEgDovJxw1
S60AoM/nHAd+FGaUCxddxaCd9V0NC2UM
=81Sq
-----END PGP SIGNATURE-----

> I just want "make bazaar not annoying for people who work on projects
> that use older formats" -- but that seemed too long ;)

Fair enough. I think we should go with poolie's suggestion: a setting called "suppress_warnings" that takes a list of keywords indicating what warnings to suppress. IMO, that works well wrt forwards compatibility.

I think the code ought to look something like ...

if GlobalConfig().suppress_warning('format_deprecation'):
    return
# output the warning as before

2009/12/5 Ian Clatworthy <email address hidden>:
>
>> I just want "make bazaar not annoying for people who work on projects
>> that use older formats" -- but that seemed too long ;)
>
> Fair enough. I think we should go with poolie's suggestion: a setting called "suppress_warnings" that takes a list of keywords indicating what warnings to suppress. IMO, that works well wrt forwards compatibility.
>
> I think the code ought to look something like ...
>
> if GlobalConfig().suppress_warning('format_deprecation'):
>    return
> # output the warning as before

Just so - though actually I would be getting the configuration for the
relevant location or branch, so that people can turn them off
selectively.

It will then be much easier to standardize on guarding all warnings like this.

--
Martin <http://launchpad.net/~mbp/>

I second Martin's remark, the option should be settable in locations.conf or branch.conf.

From there it becomes less clear that suppresss_warnings = format_deprecation, ... is such a good idea as it will forbid defining some warnings at the branch level and some others at the global level or in locations.conf.

Since there are at least two ways to address that:
- make the effective list the union of all the defined lists,
- keep the most local definition (as we do for other variables).

I think users will be confused (whatever we choose), so unless there is a strong argument for using a list, I'd prefer if we avoid this route.

Ted, do you intend to continue working on that patch ?

Martin, Ian, John, any other remark ?

We had complains for the format warning for a long time, let's reach a final agreement on how we want to handle that and let's land this patch with the final tweaks.

On Mon, 2009-12-07 at 14:37 +0000, Vincent Ladeuil wrote:
> Ted, do you intend to continue working on that patch ?

I think we've gotten well beyond my Python/bazaar knowledge... if
something was decided I'd be happy to try, but I don't think that I'd be
able to implement some of the updated suggestions.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Vincent Ladeuil wrote:
> I second Martin's remark, the option should be settable in locations.conf or branch.conf.
>
>>From there it becomes less clear that suppresss_warnings = format_deprecation, ... is such a good idea as it will forbid defining some warnings at the branch level and some others at the global level or in locations.conf.
>
> Since there are at least two ways to address that:
> - make the effective list the union of all the defined lists,
> - keep the most local definition (as we do for other variables).
>
> I think users will be confused (whatever we choose), so unless there is a strong argument for using a list, I'd prefer if we avoid this route.
>
> Ted, do you intend to continue working on that patch ?
>
> Martin, Ian, John, any other remark ?
>
> We had complains for the format warning for a long time, let's reach a final agreement on how we want to handle that and let's land this patch with the final tweaks.

This is what I remember from the deprecation warnings discussions.

1) Arguably we should only be raising a deprecation warning when someone
grabs a write lock. This is to help distinguish between branches that
the user has no control over, and ones that they do. It isn't perfect,
as you may do 'bzr status' which only grabs a read branch lock, but you
do have write access. But it is a fair trade. (IMO)

2) I prefer having the ability to shut off some branch warnings, without
disabling all of them. Possibly at the level of "allow_format=knit".

As of yet, we haven't deprecated --pack-0.92 even. Much less
1.6/1.9/1.14. If we were true to form, we probably should have
deprecated them at the 2.0 switchover. And we should certainly consider
it for 2.1. (At least, --2a is ~ as much better than --pack-0.92 as
- --pack-0.92 was over --knit, and we deprecated --knit at 1.0, IIRC.)

3) I like the idea of having it per location, but (1) may take care of
most of those complaints. If it doesn't, then a locations.conf style
entry seems the most appropriate to me. (And as always, setting it in
branch.conf makes it a global setting.)

4) As for union versus intersection, versus reset... I wouldn't worry
too much. I would reset at the next layer. So that people *can* say thas
"none of my bzr branches should be in 1.9 format, but any other branches
can be."

5) I would probably put this as a
"disable_format_deprecation_warning=True", though, and not worry too
much. Otherwise I might recommend:

disable_format_deprecation_warning=knit,pack-0.92,weave,...

There again, you may not want to get warned about pack-0.92 formats, but
if you had a weave repo around, you'd like to know...

If we did go with per-format markers, then I would make sure there was
an "all".

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAksdWKIACgkQJdeBCYSNAAPlVwCgz2UCTLP7dHXcTpnDq+nf7kGR
C1AAoLZ7Nm5pGDshtmDV4K2rT1vUvtgf
=s+xe
-----END PGP SIGNATURE-----

> On further reflection, I'd like to change the setting name to
> skip_format_deprecation_warning or
> suppress_format_deprecation_warning.

I called it "format_warning" in my branch at
lp:~bzr/bzr/conditionalize-format-warning that I started up on this
2.5 years ago, when I wanted bzr to shut the hell up about my branches
not being dirstate.

AFAIR, the primary reason it wasn't mergeable was that it would cause
multiple connections in trying to read the branch.conf.

--
Matthew Fuller (MF4839) | <email address hidden>
Systems/Network Administrator | http://www.over-yonder.net/~fullermd/
           On the Internet, nobody can hear you scream.

2009/12/8 John A Meinel <email address hidden>:
> 1) Arguably we should only be raising a deprecation warning when someone
> grabs a write lock. This is to help distinguish between branches that
> the user has no control over, and ones that they do. It isn't perfect,
> as you may do 'bzr status' which only grabs a read branch lock, but you
> do have write access. But it is a fair trade. (IMO)

The concern there is that you may be pulling from something old that
makes the pull very slow, and you possibly could fix it. However,
that would probably better be addressed by adding a warning _about the
slow fetch_ and then that in turn can also be shut off if you want.

--
Martin <http://launchpad.net/~mbp/>

>>>>> "Matthew" == Matthew D Fuller <email address hidden> writes:

    >> On further reflection, I'd like to change the setting name to
    >> skip_format_deprecation_warning or
    >> suppress_format_deprecation_warning.

    Matthew> I called it "format_warning" in my branch at
    Matthew> lp:~bzr/bzr/conditionalize-format-warning that I
    Matthew> started up on this 2.5 years ago, when I wanted bzr
    Matthew> to shut the hell up about my branches not being
    Matthew> dirstate.

Ha ! I haven't the URL handy anymore, but I knew you started such
a thing ! Thanks, I've pulled that branch.

    Matthew> AFAIR, the primary reason it wasn't mergeable was
    Matthew> that it would cause multiple connections in trying
    Matthew> to read the branch.conf.

I'll have a look at that. If the concern was the connection, it
could be fixed, it it was the round-trip, well, that will be more
controversial :-/

> I'll have a look at that.

It's probably not easy to merge forward. I have vague memories of
trying a year or so ago, and the code was significantly changed.

https://code.edge.launchpad.net/~vila/bzr/deprecation-warning-preference/+merge/16233 supersedes this proposal.

The additional required changes were too much to land without an additional review, discussion should continue on the new proposal.

I'll mark this proposal as rejected but that should really be viewed as superseded.