pkg/logging: BREAKING: move to stream-based event model

LoggerEvents get on -> Stream<LogRecord> get onRecord

Removed LoggerEvents, LoggerHandlerList, LoggerHandler

BUG=

Committed: https://code.google.com/p/dart/source/detail?r=19187

[kevmoo-old, 2013-02-26 16:56:17]

Bringing pkg/logging into the future

All tests pass.

I couldn't find someone who seems to "own" this.

I don't have a commit bit, so if this LGTY, please do the honors.

Thanks.

[Siggi Cherem (dart-lang), 2013-02-26 18:51:19]

https://codereview.chromium.org/12334093/diff/2001/pkg/logging/lib/logging.dart
File pkg/logging/lib/logging.dart (right):

https://codereview.chromium.org/12334093/diff/2001/pkg/logging/lib/logging.da...
pkg/logging/lib/logging.dart:109: Stream<LogRecord> get onRecord =>
_getStream();
can we keep the old API for a little bit so we can deprecate it gradually?

@deprecated
LoggerEvents get on => ...

I'd like to give people a couple weeks to get rid of their old API.

[kevmoo-old, 2013-02-26 19:41:36]

Added back the old IMPL with a generous amount of @deprecated

I copied the old API tests to logging_test_deprecated.

Both sets of tests pass...in checked mode. :-)

[Siggi Cherem (dart-lang), 2013-02-26 21:11:23]

Thanks Kevin.

Looks great. Just a bunch of little style nits below. If you want to do those,
great, if not I can also do those fixes here when I patch it locally.

https://codereview.chromium.org/12334093/diff/6001/pkg/logging/lib/logging.dart
File pkg/logging/lib/logging.dart (right):

https://codereview.chromium.org/12334093/diff/6001/pkg/logging/lib/logging.da...
pkg/logging/lib/logging.dart:50: /** Handlers used to process log entries in
this logger. */
fix comment

https://codereview.chromium.org/12334093/diff/6001/pkg/logging/lib/logging.da...
pkg/logging/lib/logging.dart:53: @deprecated
If you don't mind - let's remove this one here. We mainly need it in the public
APIs.

This technically shouldn't matter, but turns out that the dart_analyzer right
now is printing deprecation warnings that occur anywhere. Ideally it should only
report warnings when a user makes use of something deprecated from their
package, but currently the analyzer doesn't work that way today. By removing
this @deprecated here, I think we'll get less noise if someone calls the
analyzer from the command line.

Here is the relevant bug for the analyzer
https://code.google.com/p/dart/issues/detail?id=8691

https://codereview.chromium.org/12334093/diff/6001/pkg/logging/lib/logging.da...
pkg/logging/lib/logging.dart:118: *    logger.on.record.add((record) { ... });
+ let's add a small comment here, like:

Deprecated. Use [onRecord] instead.

https://codereview.chromium.org/12334093/diff/6001/pkg/logging/lib/logging.da...
pkg/logging/lib/logging.dart:126: if(hierarchicalLoggingEnabled || parent ==
null) {
style: add space after 'if' (same here below)

https://codereview.chromium.org/12334093/diff/6001/pkg/logging/lib/logging.da...
pkg/logging/lib/logging.dart:186: /** Adds a handler to listen whenever a log
record is added to this logger. */
move this comment down to _addHandler?

https://codereview.chromium.org/12334093/diff/6001/pkg/logging/lib/logging.da...
pkg/logging/lib/logging.dart:198: @deprecated
same here and all the ones below (the user had to use 'on' to get to these APIs,
so they will get a warning anyways).

https://codereview.chromium.org/12334093/diff/6001/pkg/logging/lib/logging.da...
pkg/logging/lib/logging.dart:200: if(_deprecatedSubscriptions == null) {
style: missing space after 'if'

https://codereview.chromium.org/12334093/diff/6001/pkg/logging/lib/logging.da...
pkg/logging/lib/logging.dart:204: final sub = onRecord.listen(handler);
style: even thought it's final, we use 'var' for these local variables. 

Given that this might fit in one line with the next line, consider just merging
them.

https://codereview.chromium.org/12334093/diff/6001/pkg/logging/lib/logging.da...
pkg/logging/lib/logging.dart:210: if(_deprecatedSubscriptions != null) {
style: space after if (same below)

https://codereview.chromium.org/12334093/diff/6001/pkg/logging/lib/logging.da...
pkg/logging/lib/logging.dart:211: final sub =
_deprecatedSubscriptions.remove(handler);
style: final -> var

https://codereview.chromium.org/12334093/diff/6001/pkg/logging/lib/logging.da...
pkg/logging/lib/logging.dart:253: @deprecated
I doubt anyone will allocate these types directly (other than the APIs that are
already marked deprecated above), but if that's the reason to add these, then
let's move them to the actual methods below ('add', 'remove', 'clear', and
'record')

That way our code in logger doesn't use any deprecated API.

[kevmoo-old, 2013-02-26 21:39:00]

If you have it all local, please feel free to update it. As long as it's less
work than iterating w/ me again.

Thanks.

[Siggi Cherem (dart-lang), 2013-02-28 00:05:15]

Committed patchset #5 manually as r19187 (presubmit successful).

[Siggi Cherem (dart-lang), 2013-02-28 00:08:32]

FYI - I tried to upload the changes here, but I wasn't allowed to because Kevin
owns the original CL.

I did the changes I mentioned in the comments and renamed the test file (it must
end in _test, otherwise our bots wont run it).

You can see the final version here:

https://code.google.com/p/dart/source/detail?r=19187