SourceHorizon / logger

Small, easy to use and extensible logger which prints beautiful logs.
https://pub.dev/packages/logger
MIT License
197 stars 33 forks source link

Logger.close() should return Future<void> #20

Closed praxamarnix closed 1 year ago

praxamarnix commented 1 year ago

The implementation of Logger.close() nicely destroys the _output, but if the _output is a FileOutput, that destroy function is async void.

// logger.dart
void close() {
  _active = false;
  _filter.destroy();
  _printer.destroy();
  _output.destroy();
}
// file_output.dart
@override
void destroy() async {
  await _sink?.flush();
  await _sink?.close();
}

It would be a best practice to let Logger await the destroy function and make destroy return a Future<void>.

// logger.dart
Future<void> close() async {
  _active = false;
  _filter.destroy();
  _printer.destroy();
  await _output.destroy();
}
// file_output.dart
@override
Future<void> destroy() async {
  await _sink?.flush();
  await _sink?.close();
}

Doing so makes it possible in a unit test to await the close function before actually deleting the temporary folder.

Bungeefan commented 1 year ago

Thanks for the issue!

Yeah, I noticed that too, we should change that, however I found no way in making this an API-compatible change. We would have to raise the major version when merging this feature, therefore I postponed it.

You (and everybody else) are free to submit a pull request, in the meantime I am trying to reproduce the current issues and possibly merge a few API-compatible features before raising the major version.