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.

Sign up for GitHub

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

Jump to bottom

R4R: Split GET delegator endpoint #2543

Merged
merged 11 commits into from
Oct 23, 2018
Merged

R4R: Split GET delegator endpoint #2543

merged 11 commits into from
Oct 23, 2018

Conversation

fedekunze
Copy link
Collaborator

@fedekunze fedekunze commented Oct 20, 2018 •
edited
Loading

Closes #2258

  • Targeted PR against correct branch (see CONTRIBUTING.md)

  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.

  • Wrote tests

  • Updated relevant documentation (docs/)

  • Added entries in PENDING.md with issue #

  • rereviewed Files changed in the github PR explorer

For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@codecov
Copy link

codecov bot commented Oct 20, 2018 •
edited
Loading

Codecov Report

Merging #2543 into develop will decrease coverage by 0.1%.
The diff coverage is 37.03%.

@@             Coverage Diff             @@
##           develop    #2543      +/-   ##
===========================================
- Coverage    60.28%   60.18%   -0.11%     
===========================================
  Files          150      150              
  Lines         8589     8607      +18     
===========================================
+ Hits          5178     5180       +2     
- Misses        3067     3081      +14     
- Partials       344      346       +2

@fedekunze fedekunze added ready-for-review T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). labels Oct 20, 2018
@fedekunze fedekunze changed the title WIP: Split delegator endpoint R4R: Split delegator endpoint Oct 20, 2018
@fedekunze fedekunze added the API label Oct 20, 2018
@fedekunze fedekunze changed the title R4R: Split delegator endpoint R4R: Split GET delegator endpoint Oct 20, 2018
@fedekunze
Copy link
Collaborator Author

Please review and merge #2542 first to add the missing tests for redelegation

@cwgoes
Copy link
Contributor

cwgoes commented Oct 21, 2018

Please review and merge #2542 first to add the missing tests for redelegation

Done, this needs conflict resolution now.

alexanderbez
alexanderbez reviewed Oct 22, 2018
View reviewed changes
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

utACK pending conflict resolutions 👍

alexanderbez
alexanderbez reviewed Oct 22, 2018
View reviewed changes
Copy link
Contributor

@alexanderbez alexanderbez left a comment •
edited
Loading

Choose a reason for hiding this comment

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

utACK pending conflict resolutions 👍

@fedekunze
Copy link
Collaborator Author

I pushed a commit but the files are not being updated ...

@codecov
Copy link

codecov bot commented Oct 22, 2018 •
edited
Loading

Codecov Report

Merging #2543 into develop will increase coverage by 0.01%.
The diff coverage is 77.77%.

@@             Coverage Diff             @@
##           develop    #2543      +/-   ##
===========================================
+ Coverage    60.32%   60.34%   +0.01%     
===========================================
  Files          150      150              
  Lines         8613     8631      +18     
===========================================
+ Hits          5196     5208      +12     
- Misses        3069     3071       +2     
- Partials       348      352       +4

@fedekunze
Copy link
Collaborator Author

@cwgoes @alexanderbez fixed and added tests

alexanderbez
alexanderbez approved these changes Oct 22, 2018
View reviewed changes
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

utACK -- although it seems like CI is hanging?

@jackzampolin
Copy link
Member

jackzampolin commented Oct 22, 2018 •
edited
Loading

This gets a 👍 from me. CI hanging might be related to the GH issues earlier? Can someone kick it?

cwgoes
cwgoes approved these changes Oct 23, 2018
View reviewed changes
Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

utACK 🍾

@cwgoes cwgoes merged commit 5476b1b into develop Oct 23, 2018
@cwgoes cwgoes deleted the fedekunze/2258-split-delegator-endpoint branch October 23, 2018 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cwgoes cwgoes cwgoes approved these changes

@alexanderbez alexanderbez alexanderbez approved these changes

@ebuchman ebuchman Awaiting requested review from ebuchman

@rigelrozanski rigelrozanski Awaiting requested review from rigelrozanski

@jackzampolin jackzampolin Awaiting requested review from jackzampolin

Assignees
No one assigned
Labels
T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine).
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fedekunze @cwgoes @jackzampolin @alexanderbez