Added resource type parameter to Provider::getConnector #47
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## 4.0 #47 +/- ##
===================================
Coverage 100% 100%
===================================
Files 29 29
Lines 371 371
===================================
Hits 371 371
Continue to review full report at Codecov.
|
Bilge
changed the title
Bilge
force-pushed
the
provider-resource-type
branch
from
42147d4
to
4be5207
Compare
Bilge
commented
Nov 6, 2017
Bilge
force-pushed
the
4.0
branch
2 times, most recently
from
765de27
to
40c2815
Compare
Bilge
force-pushed
the
master
branch
3 times, most recently
from
c394a43
to
372f181
Compare
Bilge
force-pushed
the
master
branch
2 times, most recently
from
2308bb2
to
37b143d
Compare
Bilge
force-pushed
the
master
branch
9 times, most recently
from
1d42981
to
fe2764a
Compare
Bilge
force-pushed
the
master
branch
26 times, most recently
from
8df01f3
to
4cd4dbd
Compare
Bilge
force-pushed
the
master
branch
4 times, most recently
from
93ce095
to
5898b4c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following up on the TODO comment from #41, this PR doesn't pass the resource to
Provider::getConnector, but rather, just the resource's type. I don't feel comfortable passing the entire resource because the only real use-case for passing the resource is to make decisions based on its type.Passing the entire object might cause people to do questionable things with the resource so we just pass the type name as a string instead. Unfortunately, this makes type checking code a little more cumbersome to write. Instead of
$resource instanceof Foowe must writeis_a($resourceType, Foo::class, true). Moreover, we lose the ability to distinguish between difference instances ofFoo, but it is supposed no valid use cases exist for this.The use-case for passing the resource type is applications that want to distinguish between different groups of resources that the API itself does not make. That is, certain applications may wish to group certain members of a provider's resources for application-defined reasons and pass different connectors to them.
Must decide whether or not we want this before v4 is released because it's a breaking change otherwise.