mmkal / pgkit

PostgreSQL🤝TypeScript monorepo. SQL client/admin UI/smart migrator/type generator/schema inspector
https://pgkit.dev
190 stars 25 forks source link

@slonik/migrator: Incorrect default value for logger #408

Closed samuliasmala closed 2 years ago

samuliasmala commented 2 years ago

According to Slonik migrator documentation the logger default value is console, but the default parameter is actually undefined. I.e., new SlonikMigrator(); gives no output, whereas new SlonikMigrator({ logger: console }); gives proper JSON-formatted output.

Two questions:

  1. Which should be corrected, the implementation or documentation? I assume it's the implementation but wanted to confirm this.
  2. Do you see beneficial to add a fifth configuration parameter prettyLogger: boolean which would convert JSON-logs to strings? Since the migrate.js is used from CLI that would often be useful.

I'd be happy to provide pull requests for both of the above

mmkal commented 2 years ago

Hi @samuliasmala thanks for the issue and the offer! My preference would be to update the docs rather than the default so that it's non breaking. Re 2) - I wouldn't want that to be a separate configuration parameter, but maybe the library could export a prettyLogger object which users could set as the value for logger if they want to.

samuliasmala commented 2 years ago

Thanks for the quick reply. Makes sense to update docs, I'll create a pull requests for both 1 & 2.