tbroyer / gradle-errorprone-plugin

Gradle plugin to use the error-prone compiler for Java
Apache License 2.0
366 stars 32 forks source link

documentation #55

Closed xenoterracide closed 3 years ago

xenoterracide commented 3 years ago

The following properties in kotlin seem incomplete.

first, this doesn't work anymore

    disableWarningsInGeneratedCode = true

you have to do this because now it's a Property<Boolean>

    disableWarningsInGeneratedCode.set(true)

ok, these don't appear to have docs on the properties, I'm not certain if you add javadoc/whatever to them in your plugin will it propagate to the generated kotlin dsl? imho, that would be preferred for all the things, if it doesn't work I may file or look for a bug in gradle.

what is checkOptions?

tbroyer commented 3 years ago

There's no inline documentation in the code (I must say I didn't think it would be surfaced in the IDE), but everything's documented in the README: https://github.com/tbroyer/gradle-errorprone-plugin#properties And all code snippets have a Kotlin DSL equivalent (hidden by default).

Is there anything missing?

(I'll add kdoc though)

xenoterracide commented 3 years ago

but everything's documented in the README: https://github.com/tbroyer/gradle-errorprone-plugin#properties

derp, TBH I don't even know what checkOptions is even after reading it's description, could use an example.

tbroyer commented 3 years ago

but everything's documented in the README: https://github.com/tbroyer/gradle-errorprone-plugin#properties

derp, TBH I don't even know what checkOptions is even after reading it's description, could use an example.

This is what ErrorProne actually calls a custom flag: https://errorprone.info/docs/flags#pass-additional-info-to-bugcheckers Just like you usually populate the checks map through the check() method (or more likely error(), warn(), enable() and disable()), you'd usually populate checkOptions through the option() method.

Not sure how to document that better (other than adding @see options to kdoc, and possibly an equivalent in the README form of the documentation); maybe add a note that this will map to -XepOpt:<key>=<value> for each map entry? On the other hand, one could say that if you don't know what it's for, it's probably that you don't need it :wink:

xenoterracide commented 3 years ago

Not sure how to document that better (other than adding @see options to kdoc, and possibly an equivalent in the README form of the documentation); maybe add a note that this will map to -XepOpt:= for each map entry?

either would work I think

On the other hand, one could say that if you don't know what it's for, it's probably that you don't need it 😉

how would you know if you need it if it's not documented? and you can't google it, I tried.

tbroyer commented 3 years ago

On the other hand, one could say that if you don't know what it's for, it's probably that you don't need it wink

how would you know if you need it if it's not documented? and you can't google it, I tried.

What I mean is, the moment you need to use an -XepOpt: (such as for https://errorprone.info/bugpattern/MissingOverride), you'll ask "how do I pass an -XepOpt:?" and then you will probably find the answer as option(…) or checkOptions.put(…), or maybe you'll just use errorproneArgs and that's OK too.

I'm working on adding the kdoc, then will improve the README too.

tbroyer commented 3 years ago

Hey @xenoterracide, would you mind reviewing #56 in case anything could be improved?

xenoterracide commented 3 years ago

left a comment, found a typo, don't necessarily see anything that stands out other than that.

xenoterracide commented 3 years ago

I know this is closed, but had an idea, could you document how to change the documentation of the custom task to run when check is run? I'm still considering making a custom task.