Reporter

[brettz9]

Builds on #66.

• feat: replace render with reporter array
• test: Add nyc for tests
• test: 100% coverage
• test: Use non-deprecated form file in tests

BREAKING CHANGE:

Removes render.

Also adds sourcemaps and bumps some devDeps and drops unneeded ones

(Re: the extra Rollup file, I'd like to use that as I've mentioned this on rollup/rollup#3445 , but not clear that this is possible at this point.) Removed.

[brettz9]

I've now removed the extra Rollup file, with this being expected to be entirely ready for review.

[ritz078]

I don't think we should remove render straightaway. We should keep it and show a deprecation warning and then we can remove it in the next major release. wdyt ?

[brettz9]

Sure, sounds good to me. Note that this would be a breaking change relative to recent commits as far as reporter behavior, as if the user is specifying their own already, it would no longer use the boxen reporter too. However, the reporter behavior was not released, so as long as this merge is handled before any release, it should not be a breaking change (as I may have a chance to add back the render).

[brettz9]

I've updated to be backward-compatible in supporting an explicit render (as well as the default of omitting it). I also added a test and updated the docs accordingly. Would still need to investigate the testing issue.

[brettz9]

I've now gotten the tests working here as well; it was an issue with the fact that the machines in use on Github do not support Terminal colors. The tests are now checking for environment support before making assertions on color.

[brettz9]

Oh, and I switched from colors to colors/safe to avoid colors extending the prototype (we weren't using that anyways).

[ritz078]

I will review this PR once queries in #66 are resolved.

[brettz9]

I can see about making changes to support the feature you wish to add in #66, but #66 is just documenting existing behavior, and this PR, while being a breaking change, is unrelated to that "before" size feature.

Since this PR also adds testing and full coverage, I would very much appreciate your reviewing these PRs now. Any changes I then make to add the feature to check the old npm package size (and setting it as the default per your wishes) can ensure there are no regressions.

[ritz078]

Great job 🎉 Especially the tests.

Can you run prettier on the whole codebase once. We can add a pre-commit hook later to run prettier on the staged files directly.

[ritz078]

Should we use https://github.com/babel/babel/tree/master/eslint/babel-eslint-parser instead? babel-eslint is now deprecated.

[brettz9]

Where do you see notice about babel-eslint being deprecated? I am not seeing it as such with "check-is-deprecated" or on searches. But if it does the same job, I guess it probably is better to use something under the @babel umbrella.

[brettz9]

See comment below.

[brettz9]

Re: prettier, sure, I've added a commit to apply.

(Note that even with prettier 2.0 part of package.json, we're still waiting on prettier/eslint-plugin-prettier#281 to ensure our IDEs can enforce that. But running prettier works.)

I added .prettierignore to avoid applying to .nyc_output and dist but applying to .github. Let me know if you want this some other way.