search

smhg / gettext-parser

Parse and compile gettext po and mo files, nothing more, nothing less
MIT License
158 stars 43 forks source link

feat: coverage script using nyc #78

Closed Lewiscowles1986 closed 1 year ago

Lewiscowles1986 commented 1 year ago

Just a tiny documentation script to show people how to generate coverage (can help when submitting patches)

smhg commented 1 year ago

Thank you for your contribution! I'm in favor of promoting code coverage but I'm not sure nyc is the way to go? Is it (and istanbul) still maintained? I have no idea what is the current best practice to be honest. Also, it shouldn't be run through npx but rather installed as a dev depencency I think? Based on this, I don't think this PR should be merged. But feel free to disagree - I'm happy to be proven wrong.

Lewiscowles1986 commented 1 year ago

Oh I didn't raise the PR to prove anyone wrong.

Do you have an alternative solution you prefer, which I might supplement nyc (with it's binding to istanbul) for?

Here is a package visualisation (why I did not add to package.json)

These all unfortunately seem to use istanbul 😬 and are quite large. As it's only being used to monitor a process, I thought it's so black-box locking might be over the top.

Istanbul also seems to recommend their own nyc on their homepage for mocha https://istanbul.js.org/ link

Looking at https://github.com/istanbuljs, it does look like it has been updated very recently, along with nyc and other packages.

smhg commented 1 year ago

I have to be honest: seeing npx --yes made me suspicious about a malicious contribution. Also, I'm not very up to date about code coverage. It looks like I was 100% wrong, but I'd like to be conservative in this case. I don't see enough value in this PR to move ahead with it. I'm sorry - it seems a valuable idea to add code coverage. Maybe later.

Lewiscowles1986 commented 1 year ago

👍