pq / logs

A lightweight logging framework for Flutter
29 stars 10 forks source link

improve log API #19

Open pq opened 6 years ago

pq commented 6 years ago

Follow-up from https://github.com/pq/logs/pull/16#issuecomment-425499797, minimally we should add:

@devoncarew: would 2 calls be sufficient?:


void log(MessageCallback messageCallback, {Object data, toEncodable(object)});

void logError(MessageCallback messageCallback, {Object error, toEncodable(object), StackTrace stackTrace});

Or do you think there's benefit in making a distinction between log and logData?

devoncarew commented 6 years ago

W/ logData, we could tell the API user that there were some conventions about how to transmit the data; that we'd expect it to be jsonable, and would send over the object encoded as a json string.

Putting the data object as a json encoded string in the error: field is pretty prescriptive however. Perhaps have log() - that can take an optional data: param - and have a separate logDataAsJson(), which does the prescriptive encoding?

void log(MessageCallback messageCallback, {Object data, toEncodable(object)});

looks good!

void logError(MessageCallback messageCallback, {Object error, toEncodable(object), StackTrace stackTrace});

perhaps drop the toEncodable here? I think in this case we would not try and encode the object, but would just pass the reference as is into the dart:developer call.

pq commented 6 years ago

What do we know about constraints on the error object in the developer:log API? I was working on the assumption that the error object would ultimately be encoded as json but realize now that may not be right!

devoncarew commented 6 years ago

It's just sent as a VM protocol reference - you can then call service protocol methods to get more info about it. From the client, you basically just see the vm service protocol id for the object (foo/123).

pq commented 6 years ago

20 adds data logging but we'll want to keep iterating.

food for thought from @devoncarew on that PR:

  • a dedicated logData call could be useful, esp. to indicate that we have a prescriptive way to write the data
  • if we're very perceptive of what kind of data we can log (Map<String, Object>?), I wonder if we need both LogDataCallback data and ToJsonEncodable toJsonEncodablecallbacks? The LogDataCallback could just return a Map, and we'd document that all the objects in it needed to be jsonable (or, string, maps, lists, and ints)?

👍

pq commented 6 years ago

We should hammer out what we minimally need for 1.0.

38 adds params to allow for passing of level and stackTrace down to the developer calls.

What else (if anything) do we need for v1?

/cc @devoncarew