Opened 4 years ago

Last modified 22 months ago

#4584 task new

Deprecate passing multiple positional arguments to twisted.python.log.msg

Reported by: exarkun Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: thijs Branch:
Author: Launchpad Bug:

Description

Currently this behavior is supported:

    log.msg("hello", "world")

to mean the same thing as:

    log.msg("hello world")

This complicates textFromEventDict somewhat, and makes things like #989 harder to implement as well (since it opens the possibility for mixed str and unicode and non-str non-unicode in one message).

Change History (8)

comment:1 Changed 4 years ago by glyph

I don't like that the motivation for this ticket is simply that it's hard to maintain.

For one thing, it's not that hard to maintain. But more importantly, I don't think we should drop features that are hard to maintain just because of maintenance difficulties; we should drop features which are problematic or mis-designed and also hard to maintain, because they're not worth the maintenance effort.

This feature isn't exactly the best feature in the world, but it is occasionally convenient. It lets you avoid an extra manual bit of string formatting if you've already got a string you want to use. Plus, it lets you change "print "hello", "world"" to "log.msg("hello", "world")" in fewer keystrokes.

On the other hand, the docstring does specifically advise the user to avoid this feature. If we decide to keep it we should probably change that. (And of course, in the future, any docstring like this that gets introduced really must come along with an actual DeprecationWarning, at the time that it's introduced since nobody reads docstrings.)

So, I'm somewhat ambivalent about actually keeping the feature, but I would really like this ticket to be about killing the feature on its own (lack of) merit. It's going to block #989 for a whole deprecation cycle anyway, no?

comment:2 follow-up: Changed 4 years ago by jknight

FWIW, nobody reads DeprecationWarnings either... (quick quiz: how do you enable them?)

comment:3 Changed 4 years ago by exarkun

Woo hoo, time to write our own warning system!

comment:4 in reply to: ↑ 2 ; follow-up: Changed 4 years ago by glyph

Replying to jknight:

FWIW, nobody reads DeprecationWarnings either... (quick quiz: how do you enable them?)

Run your code under trial. :)

Replying to exarkun:

Woo hoo, time to write our own warning system!

I think you mean boo hoo. But, we don't need our own warning system, just our own warning reporter. Which maybe could be expressed as an option to twistd, since trial already changes the default policy to show warnings? (I think even in 2.7, although I haven't tried it.)

comment:5 Changed 4 years ago by exarkun

So, anyway. I want to remove this feature because it's useless. The fact that it is also hard to maintain makes it worth my time to actually do the removal.

comment:6 Changed 4 years ago by thijs

  • Cc thijs added

comment:7 Changed 4 years ago by <automation>

  • Owner exarkun deleted

comment:8 in reply to: ↑ 4 Changed 22 months ago by glyph

Replying to glyph:

Replying to jknight:

FWIW, nobody reads DeprecationWarnings either... (quick quiz: how do you enable them?)

Run your code under trial. :)

Replying to exarkun:

Woo hoo, time to write our own warning system!

I think you mean boo hoo. But, we don't need our own warning system, just our own warning reporter. Which maybe could be expressed as an option to twistd, since trial already changes the default policy to show warnings? (I think even in 2.7, although I haven't tried it.)

Nope. It doesn't. And now there's a ticket about it.

Note: See TracTickets for help on using tickets.