Winson 3 support - Fixes #12 #14

halkeye · 2018-10-24T00:09:04Z

I created a new file called mixin, which wrapps around a transport and does all the formatting just for that transport

This way it can be used for console logging, or file logging, or anything else the old one did.

Totally open to feedback

rmharrison · 2018-10-31T04:05:56Z

Looks like the CI (v8, v10) is failing because updated lock files weren't commited along with the version bumps in package.json

v4 is failing because winston@3 no longer supports v4. Must be >6.4.0.

- v8, v10: Failed because `package-lock.json` wasn't in sync with `package.json` - v4: Fails because winston@3 is >v6.4.0

rmharrison · 2018-10-31T04:22:05Z

@halkeye I tried pushing updates to your branch, but it failed

remote: Permission to halkeye/winston-console-formatter.git denied to rmharrison.
fatal: unable to access 'https://github.com/halkeye/winston-console-formatter.git/': The requested URL returned error: 403

I created a PR with the fixes on your fork: halkeye#1

rmharrison · 2018-10-31T04:24:18Z

@eugeny-dementev: Would you retire node v4 from CI now that winston@3 no longer supports it?

halkeye · 2018-10-31T04:33:50Z

good catch, i didn't even look, i got distracted

halkeye · 2018-10-31T04:38:25Z

yea, 4 is past end of life - https://github.com/nodejs/Release which was 2018-04-30

…1/files

BudgieInWA · 2018-11-02T02:34:30Z

src/mixin.js

+    delete remainingInfo.trace;
+    delete remainingInfo.message;
+    delete remainingInfo.timestamp;
+    delete remainingInfo.label;


I think level needs to be removed, too.

Easy to change. Can you tell me what behavior is happening so I can write a test for it?

Winston logs made with logger.info('blah') render as something like:

[INFO] blah - level: 'info'

👍 I totally noticed after commenting and forgot to comment again

Its fixed in the new formatter, and the test is updated

…better

rmharrison · 2018-11-03T02:13:12Z

@halkeye: Thanks for merging in the formatter vs mixin changes from #12 (comment)

@BudgieInWA: Anything blocking this PR?

BudgieInWA

Someone needs to decide where to export this winston@3 formatter, but I like hiding it away in winston-console-formatter/dist/formatter until a decision about the post-winston-3 API is made.

BudgieInWA · 2018-11-05T02:05:54Z

package.json

@@ -12,7 +15,8 @@
    "lab": "^13.1.0",
    "pre-commit": "^1.2.2",
    "prettier": "^1.8.2",
-    "sinon": "^4.1.3"
+    "sinon": "^4.1.3",
+    "winston": "^3.1.0"


I'm pretty sure winston is a regular dependency here, not a dev dependency.

Oh yea.. when I first added it, it was purely for the tests, but now its the formatter, i'll get it fixed.

BudgieInWA · 2018-11-05T02:28:04Z

src/formatter.js

+    delete remainingInfo.message;
+    delete remainingInfo.timestamp;
+    delete remainingInfo.label;
+    delete remainingInfo.level;


It looks like this project uses the spread syntax, so this file can too. e.g. this can be expressed as:

module.exports = function ConsoleFormatter({ postfix = '', prefix = '', stackTrace = true, colors = defaultErrorColors, types = yamlifyColors, }) { return format(info => { const { level, label, message, timestamp, from, stack, trace, ...remainingInfo } = info; //... }) };

I originally had it that way, but something broke, I feel like tests or something, I'll certainly try it again.

> [email protected] test /home/halkeye/go/src/github.com/halkeye/winston-console-formatter > lab -T node_modules/babel-lab test/suites/*.js -l /home/halkeye/go/src/github.com/halkeye/winston-console-formatter/node_modules/babel-core/lib/transformation/file/index.js:590 throw err; ^ SyntaxError: unknown: Unexpected token (24:66) 22 | }, options); 23 | > 24 | const { level, label, message, timestamp, from, stack, trace, ...remainingInfo } = info; | ^

Yea, the tests don't run though babel, so it doesn't transpile them, and I think Object explode is only in 10

Oh well, someone can solve that in the future of they want to :)

BudgieInWA · 2018-11-05T02:37:36Z

src/formatter.js

+const { format } = require('winston');
+
+module.exports = function WinstonConsoleFormatter(options = {}) {
+  options = Object.assign({}, options, {


Doesn't this assign override any caller-provided options?

Yep, must have been tired when i wrote it, I'll get it updated and add some tests

BudgieInWA · 2018-11-05T02:45:11Z

src/formatter.js

+    colors: defaultErrorColors,
+    types: yamlifyColors,
+  });
+  return format(info => {


This API is a little bit different to the logform API. I think we should aim to blend in with the logform loggers. For example, this is format.json:

module.exports = format((info, opts = {}) => { info[MESSAGE] = jsonStringify(info, opts.replacer || replacer, opts.space); return info; });

The object that is passed to the formatter when it is created (e.g. new transports.Console({ formatter: format.json({ space: true }) })) is passed as the second argument to all calls to your format fn.

Yea, I like that a lot

I'll make sure to refer back to https://github.com/winstonjs/logform#understanding-formats when I do it, I don't think i reviewed the docs, just copied an example

halkeye · 2018-11-05T03:33:31Z

Someone needs to decide where to export this winston@3 formatter, but I like hiding it away in winston-console-formatter/dist/formatter until a decision about the post-winston-3 API is made.

if its exported it makes it way harder

halkeye · 2018-11-05T23:07:51Z

okay, i think all the issues are handled.

site note, when formatter.js test was crashing, all tests stopped running, with no errors in CI

hopefully codecov will fix it, or maybe it should get migrated to jest or something.

BudgieInWA · 2018-11-06T03:18:50Z

src/formatter.js

+
+module.exports = format((info, options = {}) => {
+  options = Object.assign(
+    {},


Because the defaults below are in an object literal, we don't need to have an empty object literal here.

True, but this is safer, there's no way for us to modify the object passed in.

First parameter is what everything is merged into, and since it's passed by reference I always err on the side of caution

Oh shit you mean because the second one w literal. Yea yea. Could do. I didn't look at the diff close enough. I'll change it I change anything else.

BudgieInWA · 2018-11-06T03:42:20Z

okay, i think all the issues are handled.

Looks good to me.

maybe it should get migrated to jest or something.

I have had a pretty easy time with Jest, including their snapshot testing (which is what a lot of these tests are).

ninja- · 2019-04-19T08:17:31Z

any update?

alqu · 2019-07-29T08:50:24Z

What is the current status of this?

ninja- · 2019-08-28T17:33:21Z

any update?

halkeye added 2 commits October 23, 2018 17:07

Allow timestamp to be overridden so its a little easier to test

0a33982

winston 3 mixin support - fixes eugeny-dementev#12

200db0f

rmharrison added a commit to rmharrison/winston-console-formatter that referenced this pull request Oct 31, 2018

Resolve CI issues for eugeny-dementev#14

5c1d8d6

- v8, v10: Failed because `package-lock.json` wasn't in sync with `package.json` - v4: Fails because winston@3 is >v6.4.0

rmharrison mentioned this pull request Oct 31, 2018

Logger3 halkeye/winston-console-formatter#1

Closed

rmharrison mentioned this pull request Oct 31, 2018

Feature/orange 795 consolidated logging amida-tech/amida-messaging-microservice#73

Merged

package lock update

06a201a

halkeye added 2 commits October 30, 2018 21:41

removing node 4 so CI works

396cc75

fixes from https://github.com/halkeye/winston-console-formatter/pull/…

5cf2f8e

…1/files

BudgieInWA mentioned this pull request Nov 2, 2018

Support winston@3 #12

Open

BudgieInWA reviewed Nov 2, 2018

View reviewed changes

halkeye added 3 commits November 2, 2018 10:51

Switch the mixin code to a formatter to make it fit the winston apis …

a0f52fa

…better

Remove level from being outputted in the mixin

06577af

Remove the old mixin

9396806

halkeye mentioned this pull request Nov 3, 2018

A bunch of minor CI fixes #15

Open

BudgieInWA reviewed Nov 5, 2018

View reviewed changes

halkeye closed this Nov 5, 2018

halkeye reopened this Nov 5, 2018

halkeye added 3 commits November 5, 2018 13:46

winson is a regular dependancy now

7777501

Switch to the format function style of arguments

dfc288a

Fix test. Apparently recent versions break with stubbing console.write

55fa995

BudgieInWA reviewed Nov 6, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Winson 3 support - Fixes #12 #14

Winson 3 support - Fixes #12 #14

halkeye commented Oct 24, 2018

rmharrison commented Oct 31, 2018 •

edited

Loading

rmharrison commented Oct 31, 2018 •

edited

Loading

rmharrison commented Oct 31, 2018 •

edited

Loading

halkeye commented Oct 31, 2018

halkeye commented Oct 31, 2018

BudgieInWA Nov 2, 2018

halkeye Nov 2, 2018

BudgieInWA Nov 5, 2018

halkeye Nov 5, 2018

rmharrison commented Nov 3, 2018

BudgieInWA left a comment

BudgieInWA Nov 5, 2018

halkeye Nov 5, 2018

BudgieInWA Nov 5, 2018

halkeye Nov 5, 2018

halkeye Nov 5, 2018

BudgieInWA Nov 5, 2018

BudgieInWA Nov 5, 2018

halkeye Nov 5, 2018

BudgieInWA Nov 5, 2018

halkeye Nov 5, 2018

halkeye commented Nov 5, 2018

halkeye commented Nov 5, 2018

BudgieInWA Nov 6, 2018

halkeye Nov 6, 2018

halkeye Nov 6, 2018

BudgieInWA commented Nov 6, 2018

ninja- commented Apr 19, 2019

alqu commented Jul 29, 2019

ninja- commented Aug 28, 2019

Winson 3 support - Fixes #12 #14

Are you sure you want to change the base?

Winson 3 support - Fixes #12 #14

Conversation

halkeye commented Oct 24, 2018

rmharrison commented Oct 31, 2018 • edited Loading

rmharrison commented Oct 31, 2018 • edited Loading

rmharrison commented Oct 31, 2018 • edited Loading

halkeye commented Oct 31, 2018

halkeye commented Oct 31, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmharrison commented Nov 3, 2018

BudgieInWA left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

halkeye commented Nov 5, 2018

halkeye commented Nov 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BudgieInWA commented Nov 6, 2018

ninja- commented Apr 19, 2019

alqu commented Jul 29, 2019

ninja- commented Aug 28, 2019

rmharrison commented Oct 31, 2018 •

edited

Loading

rmharrison commented Oct 31, 2018 •

edited

Loading

rmharrison commented Oct 31, 2018 •

edited

Loading