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

Show field type when generating accessors #2093

Merged
merged 1 commit into from
May 23, 2022

Conversation

CsCherrYY
Copy link
Contributor

@CsCherrYY CsCherrYY commented May 18, 2022

related to redhat-developer/vscode-java#2459

Signed-off-by: Shi Chen [email protected]

Copy link
Contributor

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

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

Change looks good to me. I tried in particular to see if an unresolved type might result in some null value but seems like the type is just returned. Generics also work as expected.

I would add to GenerateAccessorsHandlerTest (either modifying existing tests or creating a new one) as typeName is not currently tested there.

After that, feel free to merge.

@CsCherrYY
Copy link
Contributor Author

CsCherrYY commented May 19, 2022

@rgrunber Sure. I'll add some checks on typeName.

@rgrunber
Copy link
Contributor

rgrunber commented May 19, 2022

I think @snjeza 's buildship update will also fix the test failure from https:/eclipse/eclipse.jdt.ls/pull/2095/files#diff-081b41239a5c9e26fef6498db45e10891589f61bfb9122f6093a8e672c40034b .

@CsCherrYY

This comment was marked as off-topic.

@CsCherrYY
Copy link
Contributor Author

@rgrunber I have rebased this branch due to the conflicts and addressed the comments.

@testforstephen testforstephen merged commit 2130d70 into eclipse-jdtls:master May 23, 2022
@testforstephen testforstephen added this to the Early June 2022 milestone May 23, 2022
@CsCherrYY CsCherrYY deleted the cs-getset-ux branch May 23, 2022 06:56
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.

3 participants