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

Fix lambda compactness regression #728

Merged
merged 2 commits into from
Apr 28, 2022
Merged

Fix lambda compactness regression #728

merged 2 commits into from
Apr 28, 2022

Conversation

carterkozak
Copy link
Contributor

This reverts #708 and #707 which made expression lambdas less
readable.

==COMMIT_MSG==
Fix lambda compactness regression
==COMMIT_MSG==

This reverts #708 and #707 which made expression lambdas less
readable.
@changelog-app
Copy link

changelog-app bot commented Apr 28, 2022

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Fix lambda compactness regression

Check the box to generate changelog(s)

  • Generate changelog entry

@bulldozer-bot bulldozer-bot bot merged commit 9c353ec into develop Apr 28, 2022
@bulldozer-bot bulldozer-bot bot deleted the ckozak/revert branch April 28, 2022 13:00
@svc-autorelease
Copy link
Collaborator

Released 2.23.0

@robert3005
Copy link
Contributor

This causes conflicts with checkstyle rules in https://github.com/palantir/gradle-baseline/ :/

@carterkozak
Copy link
Contributor Author

I verified that the test output is compatible with baseline checkstyle configuration

@robert3005
Copy link
Contributor

ok, I have an actual repro. The lambda has to be more > 10 lines. Will make a pr

@robert3005
Copy link
Contributor

after digging more the included test does not pass it. The thing that it boils down to is https://github.com/palantir/gradle-baseline/blob/2a485956a20e7f2ae9a9ddfa689255e347aa96a9/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineConfig.java#L105-L141 and our detection not being fully robust. So... my proposal here is to kill that logic I linked and make that the default checkstyle?

@gatesn
Copy link
Contributor

gatesn commented Apr 28, 2022

@carterkozak do you have an example of a less readable case?

Happy with the revert if we fix the checkstyle plugin config issue, but I also thought it looked a bit prettier!

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.

6 participants