Skip to content

New Rule: BracesOnWhenStatements with configurable parameters#5838

Merged
BraisGabin merged 12 commits into
detekt:mainfrom
VitalyVPinchuk:braces_when
Apr 20, 2023
Merged

New Rule: BracesOnWhenStatements with configurable parameters#5838
BraisGabin merged 12 commits into
detekt:mainfrom
VitalyVPinchuk:braces_when

Conversation

@VitalyVPinchuk

Copy link
Copy Markdown
Contributor

For #5133

Similar to #5700

@github-actions github-actions Bot added the rules label Feb 26, 2023
@github-actions

github-actions Bot commented Feb 26, 2023

Copy link
Copy Markdown
Messages
📖 Thanks for adding a new rule to detekt ❤️
We detekted that you added tests, to your rule, that's awesome!

We detekted that you updated the default-detekt-config.yml file, that's awesome!

Generated by 🚫 dangerJS against 70183de

@VitalyVPinchuk VitalyVPinchuk changed the title New Rule: BracesOnWhenStatements with configurable parameters New Rule: BracesOnWhenStatements with configurable parameters Feb 26, 2023
@codecov

codecov Bot commented Feb 26, 2023

Copy link
Copy Markdown

Codecov Report

Merging #5838 (0e45543) into main (d067a13) will increase coverage by 0.09%.
The diff coverage is 93.18%.

❗ Current head 0e45543 differs from pull request most recent head 70183de. Consider uploading reports for the commit 70183de to get more accurate results

@@             Coverage Diff              @@
##               main    #5838      +/-   ##
============================================
+ Coverage     84.59%   84.68%   +0.09%     
- Complexity     3776     3859      +83     
============================================
  Files           546      550       +4     
  Lines         12916    13109     +193     
  Branches       2268     2313      +45     
============================================
+ Hits          10926    11102     +176     
- Misses          861      868       +7     
- Partials       1129     1139      +10     
Impacted Files Coverage Δ
...bosch/detekt/rules/style/BracesOnWhenStatements.kt 93.02% <93.02%> (ø)
...rturbosch/detekt/rules/style/StyleGuideProvider.kt 100.00% <100.00%> (ø)

... and 59 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@BraisGabin BraisGabin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The tests are so easy to read!! 😍. I left some comments but this PR looks really really good!

@cortinico cortinico added the notable changes Marker for notable changes in the changelog label Mar 3, 2023
@cortinico cortinico added this to the 1.23.0 milestone Mar 3, 2023

@TWiStErRob TWiStErRob left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice one, a few changes are needed to make sense of consistent/multi-statement. But the majority is good! Thank you!

Comment thread detekt-core/src/main/resources/default-detekt-config.yml Outdated
…/rules/style/BracesOnWhenStatements.kt

Co-authored-by: Róbert Papp <papp.robert.s@gmail.com>
…/rules/style/BracesOnWhenStatements.kt

Co-authored-by: Róbert Papp <papp.robert.s@gmail.com>
…/rules/style/BracesOnWhenStatements.kt

Co-authored-by: Róbert Papp <papp.robert.s@gmail.com>
…/rules/style/BracesOnWhenStatements.kt

Co-authored-by: Róbert Papp <papp.robert.s@gmail.com>
@VitalyVPinchuk

Copy link
Copy Markdown
Contributor Author

Also, I can't engage this rule in detekt.yml as it fails regardless of any parameter.

@BraisGabin

Copy link
Copy Markdown
Member

Also, I can't engage this rule in detekt.yml as it fails regardless of any parameter.

I don't understand the your statment. Can you elavorate?

@VitalyVPinchuk

Copy link
Copy Markdown
Contributor Author

Also, I can't engage this rule in detekt.yml as it fails regardless of any parameter.

I don't understand the your statment. Can you elavorate?

I mean if I turn on this rule, detekt task fails. No matter what parameters I use in the rule: consistent, necessary, etc. We have to fix codebase a bit

@BraisGabin

Copy link
Copy Markdown
Member

Oh, sure. No problem about that. We can enable it later in other pr with the proper fixes.

@TWiStErRob TWiStErRob left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for the delay, move was longer than expected. This looks good now, just some final touches / simplifications. Happy to help finishing up, find me on Slack. Please look through all the unresolved conversations on the PR, there might be some hiding behind expanders.

@TWiStErRob TWiStErRob left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One last round 🏆 .
Please have a look at all (potentially hidden) unresolved conversations.

…/rules/style/BracesOnWhenStatements.kt

Co-authored-by: Róbert Papp <papp.robert.s@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

notable changes Marker for notable changes in the changelog rules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants