Skip to content

New discprov service selection - #402

Merged
raymondjacobson merged 8 commits into
rj-service-selectionfrom
rj-discprov-service-selection
May 4, 2020
Merged

New discprov service selection#402
raymondjacobson merged 8 commits into
rj-service-selectionfrom
rj-discprov-service-selection

Conversation

@raymondjacobson

@raymondjacobsonraymondjacobson commented Apr 28, 2020•
edited
Loading

Copy link
Copy Markdown
Member

diff base on #399

a bunch of the code in eth contracts for this stuff can be deleted after this PR is merged, but I want to leave that for a separate PR.

@raymondjacobson
raymondjacobsonforce-pushed the rj-discprov-service-selection branch from 7237ef0 to c71f86aCompareApril 28, 2020 02:38
hareeshnagaraj
hareeshnagaraj reviewed Apr 28, 2020
View reviewed changes

@hareeshnagarajhareeshnagaraj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

still looking through, seems to be a huge improvement already

Comment threadlibs/src/services/discoveryProvider/DiscoveryProviderSelection.js
Comment threadlibs/src/services/discoveryProvider/DiscoveryProviderSelection.js
Comment threadlibs/src/services/discoveryProvider/DiscoveryProviderSelection.js
@raymondjacobson

raymondjacobson commented Apr 28, 2020

Copy link
Copy Markdown
MemberAuthor

still looking through, seems to be a huge improvement already

Ty for thoroughness! I’m not planning to deploy this for a bit since our current stuff isn’t broken. Take your time. I’m going to test more thoroughly as well.

hareeshnagaraj
hareeshnagaraj approved these changes Apr 29, 2020
View reviewed changes

@hareeshnagarajhareeshnagaraj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minor clarification around prior valid versions, overall looks great!

@raymondjacobsonraymondjacobson mentioned this pull request Apr 29, 2020
Merged
vicky-g
vicky-g reviewed Apr 29, 2020
View reviewed changes
Comment threadlibs/src/service-selection/ServiceSelection.test.js Outdated
vicky-g
vicky-g reviewed Apr 29, 2020
View reviewed changes
Comment threadlibs/src/services/discoveryProvider/DiscoveryProviderSelection.js
vicky-g
vicky-g reviewed Apr 29, 2020
View reviewed changes
Comment threadlibs/src/services/discoveryProvider/DiscoveryProviderSelection.js
vicky-g
vicky-g reviewed Apr 29, 2020
View reviewed changes
Comment threadlibs/src/services/discoveryProvider/DiscoveryProviderSelection.js
vicky-g
vicky-g reviewed Apr 29, 2020
View reviewed changes
Comment threadlibs/src/services/discoveryProvider/DiscoveryProviderSelection.test.js
vicky-g
vicky-g reviewed Apr 29, 2020
View reviewed changes
Comment threadlibs/src/services/discoveryProvider/DiscoveryProviderSelection.js
vicky-g
vicky-g reviewed Apr 29, 2020
View reviewed changes
Comment threadlibs/src/services/discoveryProvider/DiscoveryProviderSelection.test.js
@raymondjacobson

raymondjacobson commented May 4, 2020

Copy link
Copy Markdown
MemberAuthor

@vicky-g

I pulled out the actual instantiation of the discprov class from this PR. Will submit/work on that separately. Want to merge this so we can push ahead with using ServiceSelection elsewhere.
Your comments should be addressed. PTAL

vicky-g
vicky-g approved these changes May 4, 2020
View reviewed changes

@vicky-gvicky-g left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm! thanks for addressing my comments!

@raymondjacobson
raymondjacobson merged commit fe0ccaa into rj-service-selectionMay 4, 2020
@raymondjacobson
raymondjacobson deleted the rj-discprov-service-selection branch May 4, 2020 19:18
raymondjacobson added a commit that referenced this pull request May 4, 2020
@raymondjacobson
New discprov service selection (#402)
ef8eb67
* Add DiscoveryProviderSelection
* Rebase and cleanup
* Undo comment
* Reverse check valid dp versions
* Clean up
* Revert discprov index changes
* Fix imports
* Fix api call and cleanup
raymondjacobson added a commit that referenced this pull request May 4, 2020
@raymondjacobson
Add ServiceSelection class for specific services to override (#399)
319028c
* Add ServiceSelection class for specific services to override
* Clean up
* New discprov service selection (#402)
* Add DiscoveryProviderSelection
* Rebase and cleanup
* Undo comment
* Reverse check valid dp versions
* Clean up
* Revert discprov index changes
* Fix imports
* Fix api call and cleanup
@vicky-gvicky-g mentioned this pull request May 4, 2020
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@hareeshnagarajhareeshnagarajhareeshnagaraj approved these changes
@vicky-gvicky-gvicky-g approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@raymondjacobson@hareeshnagaraj@vicky-g