-
Notifications
You must be signed in to change notification settings - Fork 130
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
Refactor ASTRequestor
to reduce the access it has to the compiler
#2637
Refactor ASTRequestor
to reduce the access it has to the compiler
#2637
Conversation
Just my thought (though I probably won't have time to do a thorough review now) : If the only reason to pass in the compilation unit resolver is for creating bindings, and if clients may want to swap that out with something else, then this seems like a nice decoupling. Is it worth being more explicit (in the javadoc) on |
Good point, I'll make the Javadoc more specific |
This reduces the access that `ASTRequestor` has to the `CompilationUnitResolver` to just the method it needs, `createBinding`. This also makes it possible to use an alternate additional binding resolver instead of hardcoding the one implemented by `CompilationUnitResolver`. Signed-off-by: David Thompson <davthomp@redhat.com>
f31c4ba
to
451b85f
Compare
@rgrunber Can you please re-review, and merge if everything is fine? |
As @rgrunber is on PTO, can @jjohnstn or @testforstephen you please have a look and merge if appropriate? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this just made minor changes to the internal implementation without affecting the API. It's safe to me.
…clipse-jdt#2637) This reduces the access that `ASTRequestor` has to the `CompilationUnitResolver` to just the method it needs, `createBinding`. This also makes it possible to use an alternate additional binding resolver instead of hardcoding the one implemented by `CompilationUnitResolver`. Signed-off-by: David Thompson <davthomp@redhat.com>
…clipse-jdt#2637) This reduces the access that `ASTRequestor` has to the `CompilationUnitResolver` to just the method it needs, `createBinding`. This also makes it possible to use an alternate additional binding resolver instead of hardcoding the one implemented by `CompilationUnitResolver`. Signed-off-by: David Thompson <davthomp@redhat.com>
What it does
This reduces the access that
ASTRequestor
has to theCompilationUnitResolver
to just the method it needs,createBinding
.This also makes it possible to use an alternate additional binding resolver instead of hardcoding the one implemented by
CompilationUnitResolver
.How to test
Run the AST Converter test suite (I'm sure this feature is used elsewhere, but this is the only place that I know uses it for sure).
Author checklist