-
Notifications
You must be signed in to change notification settings - Fork 259
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
Json output #68
base: master
Are you sure you want to change the base?
Json output #68
Conversation
This is basically my first PR, so any feedback is welcome. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
Looks good!
Indeed you need to add tests to make sure future development won't break your feature. Can you add a function at the end of tests/test_cli.py
, for instance test_run_json_output()
? Something that checks that when passing -f json
, the output is parsable using json.loads()
and matches a pre-defined dictionnary.
@@ -49,6 +50,18 @@ def parsable(problem, filename): | |||
'message': problem.message}) | |||
|
|||
@staticmethod | |||
def json_output(problem, filename): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other formats don't have _output
, you can simply name this function json(problem, filename)
;-)
"char": problem.column, | ||
"description": problem.message, | ||
"code": "YAMLLINT", | ||
"name": "YAMLLINT", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name of the tool is yamllint
(lowercase), can you keep it?
@alexkiousis Hello, are you going to bring your pull request to finish? |
is there any progress on this issue? I've come across a situation where this would be very useful to have if its possible to move this forward.. |
@MrBones757 as you can see, this PR seems abandoned. Yes it's "possible to move this forward": do you want to contribute and write a new, clean pull request? It needs to come with tests (see previous comments), and the output format would be better like that:
|
Please consider using the Codeclimate JSON syntax. Why come up with a proprietary JSON schema when a de facto standard exists? It would also be supported in GitLab. Edit: I saw there was an issue closed with regards to my suggested specific JSON schema. Why when there is an output format "github"? If the JSON output would be codeclimate style then it could be used out of the box with the GitLab cloud which is no different from GitHub. Call it "gitlab" if it makes you feel better... |
This introduces a JSON format output.
Each message is a JSON dictionary. Example output:
The output is based on Phabricator's API for linting input. I use it with this Jenkins plugin.