Skip to content

Conversation

@dessina-devasia
Copy link
Contributor

@dessina-devasia dessina-devasia commented Nov 11, 2024

Resolved inconsistent behaviour of @dependency quickfix

Fixes #784

@dessina-devasia dessina-devasia requested review from mrglavas and vaisakhkannan and removed request for TrevCraw November 12, 2024 07:22
Copy link
Contributor

@vaisakhkannan vaisakhkannan left a comment

Choose a reason for hiding this comment

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

@dessina-devasia , Could you please share the screenshot after testing with this changes in the PR?

@dessina-devasia
Copy link
Contributor Author

@dessina-devasia , Could you please share the screenshot after testing with this changes in the PR?

@vaisakhkannan please find the attached video having the latest behaviour

Screen.Recording.2024-11-12.at.2.52.39.PM.mov

Copy link
Contributor

@vaisakhkannan vaisakhkannan left a comment

Choose a reason for hiding this comment

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

Thanks , Looks good to me based on the screen recording that dessina shared.

Copy link
Contributor

@mrglavas mrglavas left a comment

Choose a reason for hiding this comment

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

@dessina-devasia I spoke with @turkeylurkey and agreed that it's fine to move forward for now because this aligns exactly with LSP4Jakarta. I believe the user experience can be improved and will open an issue against LSP4Jakarta which describes that improvement.

In future, if you are copying and/or porting code from another project you should state that somewhere in the issue or PR. This helps the reviewer understand where the code came from and I would have reviewed this PR from the perspective of aligning the behaviour with LSP4Jakarta.

Copy link
Contributor

@mrglavas mrglavas left a comment

Choose a reason for hiding this comment

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

Approving, now that I understand the provenance of the code. Thanks.

@dessina-devasia
Copy link
Contributor Author

Approving, now that I understand the provenance of the code. Thanks.

Thank you @mrglavas

@dessina-devasia dessina-devasia merged commit 14e0597 into OpenLiberty:main Nov 13, 2024
1 of 3 checks passed
@dessina-devasia dessina-devasia deleted the 1025-dependent-quickfix-inconsistency branch November 13, 2024 14:33
@dessina-devasia
Copy link
Contributor Author

@dessina-devasia I spoke with @turkeylurkey and agreed that it's fine to move forward for now because this aligns exactly with LSP4Jakarta. I believe the user experience can be improved and will open an issue against LSP4Jakarta which describes that improvement.

In future, if you are copying and/or porting code from another project you should state that somewhere in the issue or PR. This helps the reviewer understand where the code came from and I would have reviewed this PR from the perspective of aligning the behaviour with LSP4Jakarta.

Sure @mrglavas

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.

Jakarta EE: Inconsistent behaviour for @Dependent quick fix.

4 participants