Allowing more Excluded Classes for ClassFanOutComplexity

Development
#1

Hi all,

I noticed that some classes from the standard api are listed in checkstyle’s AbstractClassCouplingCheck, but not in our own set of excluded classes.

The list is: EnumSet, LinkedHashMap, LinkedHashSet, Optional, OptionalDouble, OptionalInt, OptionalLong, DoubleStream, IntStream, LongStream, Stream.

I suggest we add them to our list of excluded classes to avoid being too strict in our use of ClassFanOutComplexity.

WDYT?

ClassFanOutComplexity metric
#2

+1, thank you!

#3

+1 but I don’t think this really require a vote

As suggested by xwiki-commons/checkstyle.xml at e65989511032de161a70bedb4e292060cb9de748 · xwiki/xwiki-commons · GitHub that’s something we are supposed to do already, it’s just that we need to update it regularly, and I tend to forget to check if something changed when upgrading checkstyle.

#4

Ok so we have an upgrade issue I guess (when upgrading checkstyle, we also need to merge our changes with their changes). Maybe we should have our own java class that extends checkstyle/AbstractClassCouplingCheck.java at master · checkstyle/checkstyle · GitHub so that we can add things on top of it and so that we don’t have merges to do when upgrading? (unless there’s a way in the config to add/exclude classes without overwriting the default list).

In any case, +1 to update it.

Thanks

#5

Thanks.

Done [Misc] Update the list of ClassFanOutComplexity Excluded classes · xwiki/xwiki-commons@92cf353 · GitHub

2 Likes
#6

Hi, is there any reason for not applying this to the LTS branch? Otherwise, it becomes a pain when you have to cherry-pick something that works on master but fails the build on stable-14.10.x.

#7

Thanks for pointing that out. Indeed I don’t see any objection to applying that on other branches.

1 Like