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

Implement sealed class Lenses #3359

Conversation

maksym-moroz
Copy link
Contributor

@maksym-moroz maksym-moroz commented Jan 24, 2024

Addresses #2829
Implement and enable Lens generation for sealed class properties when these properties are abstract and without extension receiver, subclasses are data classes that override properties in the constructor.
If that's not the case emit an info log message explaining why Lens is not being generated.
Additional cleanup to target logic.

All tests pass

@maksym-moroz maksym-moroz force-pushed the maksymmoroz/features/implement-sealed-class-lenses branch from 5b97025 to fb9c6dd Compare January 24, 2024 13:55
@serras
Copy link
Member

serras commented Jan 25, 2024

@maksym-moroz can you run the spotlessApply task and re-commit?

@maksym-moroz
Copy link
Contributor Author

@serras I did just that and it didn't show any issues with build

@serras
Copy link
Member

serras commented Jan 28, 2024

The automatic Spotless commit was not working, I've done it manually. This should (hopefully) turn the build green.

Copy link
Member

@serras serras left a comment

Choose a reason for hiding this comment

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

Thanks for this great work. Not only implementing the feature, but also cleaning the code a bit on the go :)

I've left a few comments about things (mostly tests) that I think should be in the code before merging it in main.

@serras serras mentioned this pull request Jan 30, 2024
22 tasks
@serras serras merged commit 44112ec into arrow-kt:main Feb 8, 2024
10 checks passed
serras added a commit to arrow-kt/arrow-website that referenced this pull request Feb 28, 2024
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.

2 participants