Re: [Codenarc-developer] Contributing (potentially) contentious rules to CodeNarc

[Chris M. <cm...@gm...>]

Marcin,

As one of our star contributors, you get extra influence!

The *MissingOverrideAnnotation* rule sounds good in any case. I assume you
can put that in the "enhanced" ruleset, for now at least.

I don't have a big problem excluding a rule from the CodeNarc internal
ruleset; I think we already do that in a few cases. So, that is not a
concern.

The *CompilationModeNotSpecified* rule, as described, sounds a bit more
custom and specialized. I'd be more hesitant to pull that one in, unless we
can make a case for wider adoption/usage.

I have been considering implementing generic *RequiredClassAnnotation* rule
and/or *IllegalClassAnnotation* rules (and possibly also *Required/Illegal
Method* rules) that check for annotations on classes/methods. Would it be
any value to come at it from that generic point of view -- perhaps allowing
a Set of annotation names that were required? The idea would then be to use
the existing Rule configuration to apply the rule to the desired set of
classes.

Thanks.
Chris

On Fri, Mar 3, 2017 at 9:11 AM, Marcin Erdmann <mar...@pr...>
wrote:

> I'm looking into contributing two rules I'm working on at the moment. I
> see them being potentially contentious and would like to ask if they have a
> chance of being accepted before putting in the effort to submit a PR. I'm
> fairly sure that I will want to reuse them across my projects so I will
> most probably open source them stand alone if you don't see them being a
> fit for the set of core rules so don't feel bad saying no.
>
> The rules are:
>
> 1. MissingOverrideAnnotation - a rule which enforces annotating methods
> which override or implement other methods with @Override as suggested in
> "Item 36: Consistently use the Override annotation" of Joshua Bloch's
> "Effective Java". This rule requires SEMANTIC_ANALYSIS compiler phase.
>
> 2. CompilationModeNotSpecified - a rule which requires every class (apart
> from inner classes, depending on configuration) to be annotated with either
> @CompileStatic or @CompileDynamic. The rationale is that we want to compile
> statically all of our production classes (dynamic compilation of a class or
> method is allowed if explicitly enabled using @CompileDynamic). We could in
> theory use a compiler configuration to achieve that instead but support for
> these is limited in IntelliJ - configuration is applied to the whole
> project which means that we would have problems disabling static
> compilation for our tests and we clearly don't want to have different
> compilation modes between build and IDE if we were not to apply a compiler
> config in the IDE.
>
> I understand that all of the built-in rules are applied to CodeNarc's
> codebase. While the first rule might trigger a lot of violations I can see
> it being justified to add all these missing @Override annotations. The
> second rule is on the other hand highly opinionated and I don't see value
> in applying it to CodeNarc's code base. Would that mean that it's a bad
> candidate for the set of built-in rules?
>
> Thanks,
> Marcin
>
> ------------------------------------------------------------
> ------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> Codenarc-developer mailing list
> Cod...@li...
> https://lists.sourceforge.net/lists/listinfo/codenarc-developer
>
>