Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support globs and fix some issues #104

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

poppinlp
Copy link

Hi. @zaach
I use this package in my work recently. I find the open issues are also my problem. So i wanna do some fix.

  • Support globs input not only files (I found this issue which i agreed)
  • Will lint all input files not just stoped when error occur since supports globs now.
  • Use commander instead of nomnom since it's deprecated and also is for this issue.
  • Fix display errors from stdin when quiet option set from this issue.

I haven't change too much code to make it easier to accept. Wait for your reply. Thanks. :)

@poppinlp
Copy link
Author

poppinlp commented Feb 6, 2018

@zaach Hi, are you there~ :)
Seems this repo is not on maintained?

@fedorov
Copy link

fedorov commented Feb 17, 2018

I have the same issue. Time to look for another json linter ...

@poppinlp
Copy link
Author

@fedorov Bad news :(
Do you have any recommended ones?

This ensures that semver-major upgrades don't get pulled in.

This became a critical bug recently because npm decided to publish a placeholder package as nomnom@2, causing *all new installs* of jsonlint to crash.
@fedorov
Copy link

fedorov commented Feb 19, 2018

@poppinlp I am not sure about the linter, but @jcfr had that idea to use the following in QIICR/dcmqi#333, which works for me, so I am getting rid of the jsonlint dependency.

python -c "import json; json.load(open('file.json'))"

@zaach
Copy link
Owner

zaach commented Feb 22, 2018

Hey @poppinlp, I did a minor release to fix the nomnom issue. Do you mind rebasing this PR?

@poppinlp
Copy link
Author

@zaach Hi. Sorry for my late reply. I've done that rebase. :)

@poppinlp
Copy link
Author

poppinlp commented Mar 4, 2018

@zaach Hi. Dose this PR ready and should i go on other issues for this repo?

@poppinlp
Copy link
Author

@zaach
Hi. Just a friendly remind~
And if you don't mind, i'll send more PR for this repo~ :)

@aerostitch
Copy link

aerostitch commented Jul 16, 2018

It would be so nice to finally have that one merged and released! :)

@pkuczynski
Copy link

@zaach any chance to merge this in? Support for globs is really needed...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants