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

idea: second part of violations #4708

Closed
Nimfadora opened this Issue Jul 12, 2017 · 5 comments

Comments

Projects
2 participants
@Nimfadora
Contributor

Nimfadora commented Jul 12, 2017

sub-task of #4681

report on violations

violations names included:

AnonymousClassMethodCount:true Production:true
AnonymousInnerClass
AntResolveInspection
AssignmentToNull
AssignmentToStaticFieldFromInstanceMethod
ClassComplexity
ClassCoupling
ClassInheritanceDepth
CloneableImplementsClone
ConstantDeclaredInAbstractClass
DeprecatedIsStillUsed
Deprecation
FieldCanBeLocal
FieldCount
FinalClass
FinalMethod
MethodCount
NoopMethodInAbstractClass
PublicConstructor
SystemOutErr
UtilityClass
UtilityClassCanBeEnum
@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Jul 12, 2017

Contributor

SystemOutErr review can be skipped

Contributor

Nimfadora commented Jul 12, 2017

SystemOutErr review can be skipped

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 13, 2017

Member

AntResolveInspection

suppress "maven and ant are used in the same build, no options in inspection to adjust"

AssignmentToNull

suppress: "we are legacy library with log history, to much items to fix, usage of Option is not do code any better in some cases, one day in future.... "

AssignmentToStaticFieldFromInstanceMethod

try to fix.
case of FileContentsHolder suppress by javadoc annotation, this class will be removed soon.

AnonymousClassMethodCount

make for test scope limit to be 3 . It should calm down.

ClassInheritanceDepth

make a limit 6 to pass, nothing we can do with it.

FieldCount

suppress: "it it hard to follow as most cases it it configuration related fields of Checks"

MethodCount

suppress: "it it hard to follow as it is better to keep whole logic in one Check class"

ClassComplexity

suppress: "it it hard to follow as it is better to keep whole logic in one Check class, we do follow cyclomatic complexity for methods"

ClassCoupling

suppress: "we control this by checkstyle/pmd that have better suppression options"

Deprecation DeprecatedIsStillUsed

try to fix.
cases AbstractTypeAwareCheck should be suppressed by javadoc annotation

CloneableImplementsClone

activate option of it, it should fix all problems that we can inherit from other libraries.

PublicConstructor

suppress: "we do not need to force all to have static factory methods"

UtilityClass

suppress: "we are not pure object-oriented library, we like reasonable mix of styles, especially state-less and context-less"

UtilityClassCanBeEnum

suppress: "we do not like this style"

NoopMethodInAbstractClass

try to suppress on these 3 files on classes level, as we have comment that explain a reason. If not possible suppress whole rule "no options to suppress methods with comment"

FinalClass
FinalMethod

suppress: "we are library - we do this on purpose as clear signal to users"

FieldCanBeLocal

fix it.

ConstantDeclaredInAbstractClass

suppress: "we are ok with constants in classes and do not like constants in interfaces"

AnonymousInnerClass

suppress: we are ok with anonymous classes

Member

romani commented Jul 13, 2017

AntResolveInspection

suppress "maven and ant are used in the same build, no options in inspection to adjust"

AssignmentToNull

suppress: "we are legacy library with log history, to much items to fix, usage of Option is not do code any better in some cases, one day in future.... "

AssignmentToStaticFieldFromInstanceMethod

try to fix.
case of FileContentsHolder suppress by javadoc annotation, this class will be removed soon.

AnonymousClassMethodCount

make for test scope limit to be 3 . It should calm down.

ClassInheritanceDepth

make a limit 6 to pass, nothing we can do with it.

FieldCount

suppress: "it it hard to follow as most cases it it configuration related fields of Checks"

MethodCount

suppress: "it it hard to follow as it is better to keep whole logic in one Check class"

ClassComplexity

suppress: "it it hard to follow as it is better to keep whole logic in one Check class, we do follow cyclomatic complexity for methods"

ClassCoupling

suppress: "we control this by checkstyle/pmd that have better suppression options"

Deprecation DeprecatedIsStillUsed

try to fix.
cases AbstractTypeAwareCheck should be suppressed by javadoc annotation

CloneableImplementsClone

activate option of it, it should fix all problems that we can inherit from other libraries.

PublicConstructor

suppress: "we do not need to force all to have static factory methods"

UtilityClass

suppress: "we are not pure object-oriented library, we like reasonable mix of styles, especially state-less and context-less"

UtilityClassCanBeEnum

suppress: "we do not like this style"

NoopMethodInAbstractClass

try to suppress on these 3 files on classes level, as we have comment that explain a reason. If not possible suppress whole rule "no options to suppress methods with comment"

FinalClass
FinalMethod

suppress: "we are library - we do this on purpose as clear signal to users"

FieldCanBeLocal

fix it.

ConstantDeclaredInAbstractClass

suppress: "we are ok with constants in classes and do not like constants in interfaces"

AnonymousInnerClass

suppress: we are ok with anonymous classes

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Jul 13, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Jul 14, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Jul 14, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Jul 14, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Jul 15, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Jul 15, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Jul 15, 2017

romani added a commit that referenced this issue Jul 15, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 15, 2017

Member

Violations included:

please activate that list of inspections and send PR(not for merge) to generate report on TC.
After each merge to our master, update that PR with list of item that still unresolved, TC will keep report for you.

Member

romani commented Jul 15, 2017

Violations included:

please activate that list of inspections and send PR(not for merge) to generate report on TC.
After each merge to our master, update that PR with list of item that still unresolved, TC will keep report for you.

@romani romani added this to In Progress in Practice What You Preach Jul 15, 2017

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Jul 16, 2017

Contributor

@romani this task is done, for what purpose I should send PR with violations?

Contributor

Nimfadora commented Jul 16, 2017

@romani this task is done, for what purpose I should send PR with violations?

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 16, 2017

Member

ok, am closing this issue as you confirming that all are covered.
We will recheck what is not covered at end of unmbrella issue (#4681).

Member

romani commented Jul 16, 2017

ok, am closing this issue as you confirming that all are covered.
We will recheck what is not covered at end of unmbrella issue (#4681).

@romani romani closed this Jul 16, 2017

@romani romani moved this from In Progress to Done in Practice What You Preach Jul 17, 2017

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