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

Cc parent optionals fix #143

Merged
merged 5 commits into from
Dec 10, 2024
Merged

Conversation

jecede
Copy link
Contributor

@jecede jecede commented Dec 6, 2024

supports optional fields in parent classes when using the copy constructors and optional getters.

assigns a property with this.prop1 = other.getProp1().orElse(null) instead of the optional returned by the getter.

@sabomichal
Copy link
Owner

It doesn't compile. I will try to fix it.

edited the wrong part in the web editor
@jecede
Copy link
Contributor Author

jecede commented Dec 9, 2024

Sorry, i copy pasted my change in the web editor, and did this incorrectly. I moved it to the correct part of the code

@sabomichal
Copy link
Owner

The tests seems not correct, the one you modified wasn't using the getter method (no inheritance and it actually compiles ok (you have to use maven profile selftest)), so I have change it, however it is still not correct when some other options are turned on. I have pushed a branch with your fix and my modifications in this repo.

@jecede
Copy link
Contributor Author

jecede commented Dec 10, 2024

I hadn't noticed the selftest profile. Updated my change to make your tests succeed, but I do not have the oversight to see if the current testcases cover everything.

@sabomichal
Copy link
Owner

Unfortunately they do not cover everything, but this something I can not fix right away.

@sabomichal sabomichal merged commit d6ef4a3 into sabomichal:master Dec 10, 2024
2 checks passed
@sabomichal
Copy link
Owner

closes #142

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