Accept a Redact function

[mrchief]

Fixes #5

[mrchief]

@davidmarkclements Here's a simple attempt. I feel like we can do away with the function check everytime and replace it with a simple statement but can't put my finger on how. Also, feel free to add/modify test names, readme changes etc. Hope this helps.

[davidmarkclements]

on top of the changes we also need a comparison benchmark

easiest way to do this would be to copy bechmarks/index.js - perhaps into benchmarks/static-censor.js and benchmarks/functional-censor.js and modify the files to use static and functional benchmarks

[davidmarkclements]

oh could you also please run a comparison benchmark against master and post results? to see the costs of the checking?

[yellowbrickc]

Hi, can I help somehow to get this pull request merged? It is just was I was looking for!

[mrchief]

@yellowbrickc If you can help fix the review comments (adding a readme example, adding a test and benchmarks), it'd be great. I got pulled into other projects and haven't had the chance to revisit this.

[yellowbrickc]

I am pretty sure that I cannot fork your changes. The only thing what I can do is to fork the repo, reapply your changes and fill the gaps. Or is there some other way I don't know about?

[yellowbrickc]

@mrchief everything is cool, I forked your fork ;)

[mrchief]

Oh Ok. I added you as collaborator to my fork also. If you push to that, it'll update this PR. If not, you'll need to create a new PR from your fork.

[davidmarkclements]

hey @yellowbrickc thanks for picking this up!

benchmark looks good, could you please run the benchmark and post the results here?

we also need to add a test for line 49 in index.js

[davidmarkclements]

great work @yellowbrickc - one nit pick in package.json

thanks for confirming on the test coverage,

all we need now is to see any performance impact, could you please run the benchmarks and then post the output in this PR thread?

[yellowbrickc]

Sure, I can do this tomorrow morning. I was wondering about 3 things, maybe you could give me some insights:

• you are using exclusively vars. As i understand how the linking is happening don't you think that const and let would be better?
• Second question: why do you never finish a function or expression with a semi-colon ?
• Third question: on which node-version should the benchmark run?

[davidmarkclements]

hey @yellowbrickc

• this was first written when const wasn't supported extensively, I'd like to keep with var for certain libraries – pino-noir is for use with pino, and pino still has a legacy branch for Node 0.10. As for let I tend to ignore it, from practice it seems that most times when block scoping is useful, so is immutable binding (const) (also out of habit, var used to be (may still be) faster than let.
• all of the official pino ecosystem modules use Standard as the linter – no semi-colons required
• latest is fine :) (8.9.4)

[mcollina]

Using let generate slower code than var. This is also true for const. Future V8 versions will fix this.

[yellowbrickc]

Thanks for the insights :)

Latest benchmark:

benchPinoTopLevel10000: 333.779ms benchNoirTopLevel10000: 382.708ms benchPinoNested10000: 491.312ms benchNoirNested10000: 481.082ms benchPinoDeepNested10000: 467.695ms benchNoirDeepNested10000: 500.071ms benchPinoVeryDeepNested10000: 518.262ms benchNoirVeryDeepNested10000: 634.933ms benchPinoWildcardStructure10000: 744.625ms benchNoirWildcards10000: 610.159ms benchPinoFunctionCensor10000: 334.572ms benchNoirFunctionCensor10000: 459.203ms benchPinoTopLevel10000: 265.533ms benchNoirTopLevel10000: 329.132ms benchPinoNested10000: 397.983ms benchNoirNested10000: 401.191ms benchPinoDeepNested10000: 397.944ms benchNoirDeepNested10000: 489.064ms benchPinoVeryDeepNested10000: 509.982ms benchNoirVeryDeepNested10000: 643.705ms benchPinoWildcardStructure10000: 582.878ms benchNoirWildcards10000: 564.312ms benchPinoFunctionCensor10000: 328.049ms benchNoirFunctionCensor10000: 424.109ms

[yellowbrickc]

Hi @davidmarkclements do you have an idea when you will merge and release this pull request?

[davidmarkclements]

Sorry! This fell of my radar - hopefully tomorrow or thu at latest

On Tue, 6 Mar 2018 at 21:36, Krisztina Hirth notifications@github.com wrote:

Hi @davidmarkclements https://github.com/davidmarkclements do you have an idea when you will merge and release this pull request?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pinojs/pino-noir/pull/7#issuecomment-370936931, or mute the thread https://github.com/notifications/unsubscribe-auth/ABIrPKOKVhsfCTM3O6uK6su4UNOE6Zw1ks5tbwFRgaJpZM4QmTY4 .

[davidmarkclements]

released v2.2.0

really great collab. @yellowbrickc and @mrchief