#26 Switch URI look up to be based on the container's value, not HTTP… #29
Conversation
johnament
force-pushed
the
uri-injection-refactor
branch
3 times, most recently
from
2ef2045 to
90fb0b2
Compare
|
@bartoszmajsak ping? |
|
|
||
| import java.lang.annotation.Annotation; | ||
|
|
||
| public final class ArquillianResourceLiteral implements ArquillianResource { |
There was a problem hiding this comment.
Does it need to be public?
There was a problem hiding this comment.
Probably not - but may be could also be added to core?
| Collection<ResourceProvider> resourceProviders = loader.get().all(ResourceProvider.class); | ||
| for(ResourceProvider resourceProvider: resourceProviders) | ||
| { | ||
| if(resourceProvider.canProvide(URI.class)) |
There was a problem hiding this comment.
What if we can have more than one provider? This will fail on the first one returning null but maybe there is another one which can pass? Just speculating
There was a problem hiding this comment.
The same issue is in core today. If i have multiple providers for URI, I simply get the first. If you recall, I pinged you a while ago about disabling the built in one, which lead to this little hack
There was a problem hiding this comment.
@johnament would that help? arquillian/arquillian-core#118 -> https://issues.jboss.org/browse/ARQ-2053, it's in latest Core.
In fact it was already available long before, as you can read from the discussion on the PR arquillian/arquillian-core#118 (comment), but well hidden :)
There was a problem hiding this comment.
@johnament did you have a chance to look at this feature in Core? Would it simplify this PR? I think it's time to merge it :D
There was a problem hiding this comment.
@bartoszmajsak I read through those (I think I read them before). It's not clear to me based on the comments in that thread what I need to change here.
|
If you're going to do a release soon, could you also address #20, perhaps by adding the new and deprecating the old? |
@mojavelinux I would just rename. It's a simple change and will enforce to change immediately. I think cost for such a change is quite low. |
|
@bartoszmajsak long time no see on this one. Do we still need it? |
…, not HTTPContext. Fixed shrinkwrap version.
johnament
force-pushed
the
uri-injection-refactor
branch
from
90fb0b2 to
961b9a1
Compare
…Context. Fixed shrinkwrap version.
This change is