Skip to content

Conversation

@joshkgold
Copy link
Contributor

Based on the popular Checkstyle format, allow lints to be exported to an xml file. Useful for CI servers like Jenkins that can display a Checkstyle report and visualize errors.

Note this is the only bit of R code I've written, out of necessity. Comments and critiques on style and semantics are appreciated.

@jimhester jimhester merged commit 8dbac39 into r-lib:master May 24, 2016
@jimhester
Copy link
Member

Thanks for the PR!

I ended up rewriting the majority of this, but your code gave me a good idea of the requirements. The major things I changed were

  • Uses xml2 to create the XML rather than doing it by hand.
  • Does not try to create a results directory (this does not belong in this function IMHO)
  • More lengthy test including multiple source files in results.

Thanks again!

@joshkgold
Copy link
Contributor Author

Awesome! I don't even want to know how many minutes you spent overhauling it compared to the hours it took me to write that small piece of code :-)

Thanks for reviewing and improving!

Get Outlook for iOShttps://aka.ms/o0ukef


From: Jim Hester <[email protected]mailto:[email protected]>
Sent: Tuesday, May 24, 2016 9:02 AM
Subject: Re: [jimhester/lintr] Allow lint results to be exported as checkstyle xml (#156)
To: jimhester/lintr <[email protected]mailto:[email protected]>
Cc: Gold, Joshua K <[email protected]mailto:[email protected]>, Author <[email protected]mailto:[email protected]>

Thanks for the PR!

I ended up rewriting the majority of this, but your code gave me a good idea of the requirements. The major things I changed were

  • Uses xml2 to create the XML rather than doing it by hand.
  • Does not try to create a results directory (this does not belong in this function IMHO)
  • More lengthy test including multiple source files in results.

Thanks again!


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHubhttps://github.com//pull/156#issuecomment-221261331

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.

2 participants