Skip to content
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

Reducible #215

Merged
merged 10 commits into from
Aug 20, 2017
Merged

Reducible #215

merged 10 commits into from
Aug 20, 2017

Conversation

JorgeCastilloPrz
Copy link
Member

Fixes #211

  • Added Reducible and NonEmptyReducible instance.
  • Added Reducible laws.
  • Added tests for Reducible and ReducibleLaws.

@raulraja raulraja mentioned this pull request Aug 20, 2017
54 tasks
@codecov-io
Copy link

codecov-io commented Aug 20, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@4d6ceeb). Click here to learn what that means.
The diff coverage is 28.71%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #215   +/-   ##
=========================================
  Coverage          ?   50.44%           
  Complexity        ?      206           
=========================================
  Files             ?      101           
  Lines             ?     2266           
  Branches          ?      276           
=========================================
  Hits              ?     1143           
  Misses            ?     1011           
  Partials          ?      112
Impacted Files Coverage Δ Complexity Δ
kategory/src/main/kotlin/kategory/data/Composed.kt 32.65% <0%> (ø) 0 <0> (?)
...ategory/src/main/kotlin/kategory/typeclasses/Eq.kt 44.44% <0%> (ø) 0 <0> (?)
.../main/kotlin/kategory/instances/ListKWInstances.kt 72.41% <0%> (ø) 0 <0> (?)
.../src/main/kotlin/kategory/generators/Generators.kt 70.83% <100%> (ø) 0 <0> (?)
kategory/src/main/kotlin/kategory/data/ListKW.kt 58.33% <100%> (ø) 1 <0> (?)
kategory/src/main/kotlin/kategory/data/Option.kt 53.12% <100%> (ø) 2 <0> (?)
...y/src/main/kotlin/kategory/typeclasses/Foldable.kt 23.52% <26.31%> (ø) 0 <0> (?)
...est/src/main/kotlin/kategory/laws/ReducibleLaws.kt 3.44% <3.44%> (ø) 1 <1> (?)
.../src/main/kotlin/kategory/typeclasses/Reducible.kt 48.64% <48.64%> (ø) 0 <0> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d6ceeb...8e1f9a5. Read the comment docs.

detekt.yml Outdated
@@ -84,7 +84,7 @@ formatting:
active: true
autoCorrect: true
ExpressionBodySyntaxLineBreaks:
active: true
active: false
Copy link
Member

Choose a reason for hiding this comment

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

🎉

Copy link
Member

Choose a reason for hiding this comment

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

I'm the waterparties here. The detek baseline will need to be regenerated and the long lines fixed if this is not enforced anymore :P

Copy link
Member Author

Choose a reason for hiding this comment

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

Could enable it again, but detekt was not allowing me to compile in any way (tried many workarounds) because of format requirement collisions on 2 different rules (I think it was this one and MaxLineLength). I didn't have any other choice than disabling this.

Copy link
Member

Choose a reason for hiding this comment

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

ExpressionBodySyntaxLineBreaks is an auto-fix that's bugged. Instead of applying to every instance, it only applies to the first one per file. You have to run it multiple times to make it work.

Once those errors are gone, you'll get a lot of MaxLineLength ones. For that you just regenerate the baseline with ./gradlew detektEstablishAcceptedErrors and push

Copy link
Member

@pakoito pakoito Aug 20, 2017

Choose a reason for hiding this comment

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

The train of thought is that autofix > MaxLineLength + regenerate baseline, for consistency purposes. It's quite annoying for now, so reverting would mean fixing all the MaxLineLength back. No more than 30 minutes to do it tho.

Copy link
Member Author

@JorgeCastilloPrz JorgeCastilloPrz Aug 20, 2017

Choose a reason for hiding this comment

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

Ok. I've done what we agreed on in slack for this PR:

  • Auto-fixed ExpressionBodySyntaxLineBreaks violations with ./gradlew detekt (running it multiple times).
  • When I just had a few MaxLineLength violations I regenerated baseline with ./gradlew detektEstablishAcceptedErrors.

@JorgeCastilloPrz JorgeCastilloPrz merged commit c723d04 into master Aug 20, 2017
@JorgeCastilloPrz JorgeCastilloPrz deleted the jorge-reducible branch August 20, 2017 17:16
rachelcarmena pushed a commit that referenced this pull request Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants