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

Add support for 'console' output type of CodeNarc plugin (Fix 1481) #2170

Merged
merged 3 commits into from Jun 8, 2017

Conversation

blindpirate
Copy link
Collaborator

@blindpirate blindpirate commented May 28, 2017

Context

See #1481 and discussion in old forum.

Although it is documented that CodeNarc plugin supports 'console' output type, that type is never implemented. This commit add 'console' TaskGeneratedSingleFile to CodeNarcReportsImpl, and adjust the options with which Ant codenarc task is called. To redirect Ant output to Gradle standard output in 'console' type, the lifecycleLogLevel of AntLogAdapter is changed to INFO.

Then, a unit test and an integration test are provided.

Contributor Checklist

  • Review Contribution Guidelines
  • Sign Gradle CLA
  • Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective
  • Provide unit tests (under <subproject>/src/test) to verify logic
  • Ensure that tests pass locally: ./gradlew quickCheck <impacted-subproject>:check

This fix gradle#1481 . Although it is
documented that CodeNarc plugin supports 'console' output type, that
type is never implemented. This commit add 'console'
TaskGeneratedSingleFile to CodeNarcReportsImpl, and adjust the options
with which Ant codenarc task is called. To redirect Ant output to Gradle
standard output in 'console' type, the lifecycleLogLevel of AntLogAdapter
is changed to INFO.

Then, a unit test and an integration test are provided.
static void setLifecycleLogLevel(Object ant, String lifecycleLogLevel) {
ant?.builder?.project?.buildListeners?.each {
// We cannot use instanceof or getClass()==AntLoggingAdapter since they're in different class loaders
if (it.class.simpleName == 'AntLoggingAdapter') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there is no simpler way to do this.

We can improve it slightly, by replacing the check with
it.class.name == AntLoggingAdapter.name

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right.

option(name: 'outputFile', value: r.destination)
// See http://codenarc.sourceforge.net/codenarc-TextReportWriter.html
if (r.name == 'console') {
setLifecycleLogLevel(ant,'INFO')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following Gradle's code style rules, this should be (ant, 'INFO') <- with a space

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. You're so careful.

option(name: 'writeToStandardOut', value: true)
}
} else {
setLifecycleLogLevel(ant,null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(ant, null)

@jjohannes
Copy link
Contributor

@blindpirate PR looks good, thanks! Just some minor remarks. Could you address them? Then this can go into 4.1

I think this also deserves a short mentioning in the Release Notes. Could you add a ### section with one or two sentences in the ## New and noteworthy section as part of the PR?
subprojects/docs/src/docs/release/notes.md
Don't worry too much about wording. I'll polish it if needed.

@blindpirate
Copy link
Collaborator Author

As you say, I have improved it. Please let me know if there're any other questions.

@jjohannes jjohannes merged commit 8be387d into gradle:master Jun 8, 2017
@jjohannes jjohannes added this to the 4.1 RC1 milestone Jun 8, 2017
@jjohannes
Copy link
Contributor

Thanks @blindpirate! The PR is merged and will be part of the 4.1 release.

@blindpirate
Copy link
Collaborator Author

Thanks @jjohannes .

@blindpirate blindpirate deleted the codenarc-console branch June 8, 2017 07:29
@jjohannes jjohannes modified the milestones: 4.1 M1, 4.1 RC1 Jun 20, 2017
@vgaidarji
Copy link

@blindpirate did you consider implementing support for multiple reports?
It might be useful to have both 'console' and 'html' reports generated, so that some people can check console for errors, others will open nice visual HTML report somewhere in CI.
How difficult it will be to support multiple reports?

@blindpirate
Copy link
Collaborator Author

@vgaidarji Right now I don't think have enough bandwidth/priority to implement such a feature. However, I think this shouldn't be too difficult, maybe at most 100 lines change? We're glad if any contributions can be made.

@twilco
Copy link

twilco commented Feb 14, 2019

So just to confirm, multiple report format outputs aren't currently supported with this plugin? I have to choose either console, or HTML, or XML?

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

Successfully merging this pull request may close these issues.

None yet

5 participants