Skip to content

More precise language for InvocationContext.getInterceptorBindings()#103

Merged
starksm64 merged 1 commit intojakartaee:masterfrom
Ladicek:interceptor-bindings-improved
Nov 30, 2023
Merged

More precise language for InvocationContext.getInterceptorBindings()#103
starksm64 merged 1 commit intojakartaee:masterfrom
Ladicek:interceptor-bindings-improved

Conversation

@Ladicek
Copy link
Copy Markdown
Member

@Ladicek Ladicek commented Jul 18, 2023

No description provided.

@Ladicek
Copy link
Copy Markdown
Member Author

Ladicek commented Jul 18, 2023

When I was working on the TCK tests for InvocationContext.getInterceptorBindings() (jakartaee/cdi-tck#479), I realized there are some "interesting" cases that the previous specification didn't exactly cover well (allowed multiple interpretations). In this PR, I'm trying to tighten up the language to be precise.

manovotn
manovotn approved these changes Jul 18, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@manovotn manovotn left a comment

Choose a reason for hiding this comment

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

LGTM

@manovotn
Copy link
Copy Markdown
Contributor

manovotn commented Jul 18, 2023

Also CC @arjantijms for committer review :)

@arjantijms
Copy link
Copy Markdown
Contributor

arjantijms commented Jul 18, 2023

LGTM, I do wonder if the specification text is still necessary when the javadoc already covers it. Though that's maybe a totally different discussion.

@arjantijms arjantijms added this to the 2.2.0 milestone Jul 18, 2023
@Ladicek
Copy link
Copy Markdown
Member Author

Ladicek commented Jul 18, 2023

The reason why I basically copy the javadoc to the specification text is that the specification text should be complete. I generally like AtInject's approach that there is no other specification text than javadoc, but here, we have an extra specification text, and I feel like the best approach is to just duplicate the text.

@Ladicek Ladicek mentioned this pull request Jul 18, 2023
Merged
@manovotn
Copy link
Copy Markdown
Contributor

manovotn commented Sep 20, 2023

@arjantijms is there anything blocking this from getting merged?

@Ladicek Ladicek mentioned this pull request Nov 30, 2023
Merged
@starksm64 starksm64 merged commit 1f68a6b into jakartaee:master Nov 30, 2023
@Ladicek Ladicek deleted the interceptor-bindings-improved branch November 30, 2023 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arjantijms arjantijms arjantijms approved these changes

+1 more reviewer

@manovotn manovotn manovotn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.2.0

Development

Successfully merging this pull request may close these issues.

4 participants

@Ladicek @manovotn @arjantijms @starksm64