Skip to content

Revert "Enable switchBlockAsSingleDecisionPoint (#2383)"#2386

Merged
bulldozer-bot[bot] merged 1 commit intodevelopfrom
ckozak/checkstyle-revert
Sep 15, 2022
Merged

Revert "Enable switchBlockAsSingleDecisionPoint (#2383)"#2386
bulldozer-bot[bot] merged 1 commit intodevelopfrom
ckozak/checkstyle-revert

Conversation

@carterkozak
Copy link
Copy Markdown
Contributor

This reverts commit e132e2d.

==COMMIT_MSG==
Revert "Enable switchBlockAsSingleDecisionPoint (#2383)" due to configuration parsing failures.
==COMMIT_MSG==

@pkoenig10
Copy link
Copy Markdown
Member

@carterkozak What exactly is failing here? Can we just fix the config instead of reverting?

<module name="CyclomaticComplexity"> <!-- Java Coding Guidelines: Reduce Cyclomatic Complexity -->
<property name="switchBlockAsSingleDecisionPoint">true</property>
</module>
<module name="CyclomaticComplexity"/> <!-- Java Coding Guidelines: Reduce Cyclomatic Complexity -->
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Revert seems fine, but should we just fix this?

Suggested change
<module name="CyclomaticComplexity"/> <!-- Java Coding Guidelines: Reduce Cyclomatic Complexity -->
<module name="CyclomaticComplexity"> <!-- Java Coding Guidelines: Reduce Cyclomatic Complexity -->
<property name="switchBlockAsSingleDecisionPoint" value="true"/>
</module>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

also surprised a test didn't catch this, and excavator bump #2385 had to catch the failure

Caused by: org.xml.sax.SAXParseException; systemId: file:/home/circleci/project/.baseline/checkstyle/checkstyle.xml; lineNumber: 406; columnNumber: 63; Attribute "value" is required and must be specified for element type "property".

@bulldozer-bot bulldozer-bot Bot merged commit a7b9b26 into develop Sep 15, 2022
@bulldozer-bot bulldozer-bot Bot deleted the ckozak/checkstyle-revert branch September 15, 2022 16:39
@svc-autorelease
Copy link
Copy Markdown
Collaborator

Released 4.166.0

@carterkozak
Copy link
Copy Markdown
Contributor Author

Can we just fix the config instead of reverting?

My preference is to get back to a known good state asap, unrevert+fix isn't substantially more difficult after the revert has been rolled out, but it means if the second attempt doesn't work, we don't have to do commit archeology to get ourselves back to a stable place.

I didn't do enough testing when I approved the earlier PR, so I'd like to ensure I have the time to thoroughly test before we fix forward, and I don't have availability in the next couple hours. I'd really like to see an integration test that prevents this sort of thing moving forward.

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.

5 participants