Dropping args and replacing by minimist

pinojs / pino-pretty

🌲Basic prettifier for Pino log lines

MIT License

1.27k stars 150 forks source link

Dropping args and replacing by minimist #350

Closed marcelfranca closed 2 years ago

marcelfranca commented 2 years ago

Fix issue https://github.com/pinojs/pino-pretty/issues/336

marcelfranca commented 2 years ago

I might understood wrong but I was told to create a DRAFT as soon as I started to work to show that someone is working on it.

simoneb commented 2 years ago

@jsumners we are used to opening Draft PRs early on, no need to request changes on a Draft PR anyway since it can't be merged.

@marcelfranca you mentioned you're having troubles migrating some options from args to minimist, shout if you need any input so the people contributing to this repo can help out.

jsumners commented 2 years ago

In the future, please start work from a feature branch. See https://jrfom.com/posts/2017/03/08/a-primer-on-contributing-to-projects-with-git/#create-a-feature-branch

marcelfranca commented 2 years ago

Yes, minimist does support, I'll add it.

Also, I created a PR for args that fixes the problem we had with it, but no updates so far.

Here is the link for the PR: https://github.com/leo/args/pull/166

jsumners commented 2 years ago

Also, I created a PR for args that fixes the problem we had with it, but no updates so far.

Here is the link for the PR: leo/args#166

You should add a test to that PR that fails prior to your fix.

simoneb commented 2 years ago

I don't like moving the help text out to a text file:

It will get out of sync

The examples are no longer contextual, they are only available in the root --help

But I'm not going to block this.

I understand that and I can agree. Unfortunately that's the only option with minimist (afaik), also used in https://github.com/fastify/fastify-cli/

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 2542653509

0 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 100.0%

Totals
Change from base Build 2542630725:	0.0%
Covered Lines:	383
Relevant Lines:	383

💛 - Coveralls

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 2542653509

7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 100.0%

Totals
Change from base Build 2542630725:	0.0%
Covered Lines:	383
Relevant Lines:	383