Skip to content
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

Remove name argument from Discovery classes #5

Closed
sagikazarmark opened this issue Jun 6, 2015 · 3 comments
Closed

Remove name argument from Discovery classes #5

sagikazarmark opened this issue Jun 6, 2015 · 3 comments
Assignees
Milestone

Comments

@sagikazarmark
Copy link
Member

As discussed in #4

Reason against it: it allows to identify a class resource and allows to override it when registering one.

/cc @ddeboer

@ddeboer
Copy link
Contributor

ddeboer commented Jun 12, 2015

I prefer to remove the names and use priorities to make overriding lookups possible. Perhaps manually register()ed classes should always go before the default ones? That’s probably sufficient.

@sagikazarmark sagikazarmark modified the milestone: v0.1 Jun 12, 2015
@sagikazarmark
Copy link
Member Author

Priority only make sense when you register your custom adapters. I guess you won't install more than one adapters. Even if you would, you shouldn't care about which exactly is instantiated.

If this is true, we should just unshift the manually registered classes => done.

@ddeboer
Copy link
Contributor

ddeboer commented Jun 12, 2015

True, just put manually registered classes on top (so unshift).

@sagikazarmark sagikazarmark mentioned this issue Jun 12, 2015
@sagikazarmark sagikazarmark self-assigned this Jun 12, 2015
sagikazarmark added a commit that referenced this issue Jun 12, 2015
Name replaced with priority (Fix #5)
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

No branches or pull requests

2 participants