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

create XSD definition for XML report #5166

Open
keradus opened this issue Oct 1, 2017 · 24 comments
Open

create XSD definition for XML report #5166

keradus opened this issue Oct 1, 2017 · 24 comments
Labels

Comments

@keradus
Copy link

keradus commented Oct 1, 2017

Hi

Checkstyle can produce a report used later in different Services, like Sonar, Jenkins and so.
Where can I find a xsd definition of that report ?

@romani romani added the approved label Oct 9, 2017
@romani romani changed the title XSD definition for report create XSD definition for XML report Oct 9, 2017
@romani
Copy link
Member

romani commented Oct 9, 2017

unfortunately there is no xsd for result xml report, probably it was just forgotten to create.
But it will be very good to have it in repo.

@keradus
Copy link
Author

keradus commented Oct 9, 2017

Thanks for an answer and approving my issue as a request 👍
For now, I'm using https://github.com/linkedin/pygradle/blob/master/pygradle-plugin/src/test/resources/checkstyle/checkstyle.xsd , it could be a nice starting point (yet for me, as the definition is unofficial, I cannot be sure it is fully proper currently and it will in future)

@romani
Copy link
Member

romani commented Oct 9, 2017

good version to start.
we need to:

  1. get approval to change a license on this file. @keradus, can you approve this ?
  2. we need to update build/deploy https://github.com/checkstyle/checkstyle/blob/master/pom.xml#L596 to publish such file automatically during release. We use dtds for some very historical reason that nobody remember now, xsd usage is more preferable now. In web we should have folder "xsd" and keep all files there.
  3. xdoc documentation need to be updated
  4. update XMLLogger to put xsd reference in result xml.

@keradus
Copy link
Author

keradus commented Oct 10, 2017

@romani , I cannot. Simply, it's not mine asset. I pointed to it as it may be a nice starting point, but it is not under my control. (I had to have xsd file and that is what I found)

also, as a first step IMO it's enough to have xsd even only in repo

@rnveach
Copy link
Member

rnveach commented Oct 10, 2017

update XMLLogger to put xsd reference in result xml.

@romani We should first verify maven-checkstyle-plugin will be ok with an XSD in the XML file.
If it won't accept it, then we can't make the change. See Issue #5168 .

@romani
Copy link
Member

romani commented Oct 10, 2017

@rnveach , good point, CI will catch that.
If there a problem, enforcing of xsd by default should be postponed till maven-checkstyle-plugin could work with it.

@keradus
Copy link
Author

keradus commented Oct 10, 2017

@romani
as a first step you could create xsd without pointing to it in generated report, it would still be a great help to integrate tools with checkstyle

@keradus
Copy link
Author

keradus commented Jan 2, 2018

ping @romani , any news on this ? :)

@romani
Copy link
Member

romani commented Jan 2, 2018

Sorry, over occupied with active PRs, if you have time, please send PR, it will speed up the fix.

@keradus
Copy link
Author

keradus commented Jan 3, 2018

I have no clue what is the report specification, that's exactly the thing I'm asking for

@rnveach
Copy link
Member

rnveach commented Jan 3, 2018

@keradus If you mean the contents of the XML file to make the schema, you can look at our test expected outputs.
https://github.com/checkstyle/checkstyle/tree/master/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger

@keradus
Copy link
Author

keradus commented Jan 4, 2018

sadly, i don't understand half of it (especially when you start raising bugs to it), be aware that I never played with checkstyle.
users of my tool wants to benefit from checkstyle report format, and I believe that checkstyle will also benefit if it would standardize it

@romani
Copy link
Member

romani commented Jan 5, 2018

users of my tool wants to benefit from checkstyle report format

please share a link to it, let us be aware of it and know how exactly it will benefit. For some legacy reasons we still use DTD , not xsd. So creation of xsd will be a bit unusual for us so please share where benefit is expected.

@keradus
Copy link
Author

keradus commented Jan 5, 2018

#5166 (comment)

the tool is PHP CS Fixer, which can report the outcome in different formats, one of requested by community is "checkstyle alike"

@keradus
Copy link
Author

keradus commented Jul 15, 2018

Hi there! any update on this one? Would be great to finally have the format standardized!

@rnveach
Copy link
Member

rnveach commented Jul 15, 2018

There are no updates as no one is working on this issue. You are welcome to help us with this.

@keradus
Copy link
Author

keradus commented Jul 16, 2018

For now, I'm using https://github.com/linkedin/pygradle/blob/master/pygradle-plugin/src/test/resources/checkstyle/checkstyle.xsd , it could be a nice starting point

as a first step you could create xsd without pointing to it in generated report

yet,

I have no clue what is the report specification, that's exactly the thing I'm asking for

@rnveach
Copy link
Member

rnveach commented Jul 16, 2018

@keradus Since you have an XSD can't you apply it to all our expected outputs from our junits to verify it works for them? Our junit results should have all the expected formats.

https://github.com/linkedin/pygradle/blob/master/pygradle-plugin/src/test/resources/checkstyle/checkstyle.xsd

https://github.com/checkstyle/checkstyle/blob/master/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger/ExpectedXMLLoggerException2.xml
It doesn't support the exception tag.

https://github.com/checkstyle/checkstyle/blob/master/src/test/resources/com/puppycrawl/tools/checkstyle/ant/checkstyleanttask/ExpectedCheckstyleAntTaskXmlOutput.xml#L4
Only column number is optional for error tag. Line, severity, message, and source are required.

https://github.com/checkstyle/checkstyle/blob/master/src/test/resources/com/puppycrawl/tools/checkstyle/xmllogger/ExpectedXMLLoggerErrorModuleId.xml
It doesn't support errors outside a file tag. Not sure how accurate this is that we print violations on a non-file, but it could happen if things aren't coded properly I'm sure.

I'm not seeing anything wrong with it, but I've never worked with XSD much. Running against our junit results would be the best way to verify it.
Let me know if there is anything else you need.

@keradus
Copy link
Author

keradus commented Jul 16, 2018

Let me know if there is anything else you need.

basically that would be... any java skills? ¯_(ツ)_/¯
as I revealed, no experience with java, thus no experience with checkstyle project as well.

let my say it once more, multiple CIs, like Jenkins, Sonar and so, may want to display report of checkstyle, but without knowing it's format, we always hit some edge cases that sth is there or not, because of assumptions that were made, because of missing spec

@romani
Copy link
Member

romani commented Jul 16, 2018

but without knowing it's format, we always hit some edge cases that sth is there or not, because of assumptions that were made, because of missing spec

Ok, looks like we do not have neither DTD not xsd for report, but have full coverage over configuration files https://github.com/checkstyle/checkstyle/tree/master/src/main/resources/com/puppycrawl/tools/checkstyle .
DTD or XSD need to be created.

@romani
Copy link
Member

romani commented Jul 17, 2018

@keradus , are be ok with DTD ? in report

@keradus
Copy link
Author

keradus commented Jul 17, 2018

I, personally, would vote for XSD, if you ask me.

Yet, ultimately, any standard scheme is good for that, DTD will work for me for sure 👍

@romani
Copy link
Member

romani commented Jul 18, 2018

@subkrish , can you help us with DTD creation for result XML report ?
we also need to update XMLLogger.java to print reference to DTD in xml file.

@subkrish
Copy link
Contributor

@romani @rnveach

Sure. I'll work on it.

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

No branches or pull requests

4 participants