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

Support batched votes #240

Merged
merged 9 commits into from
May 16, 2018
Merged

Support batched votes #240

merged 9 commits into from
May 16, 2018

Conversation

mrose17
Copy link
Contributor

@mrose17 mrose17 commented May 9, 2018

Resolves #212

adds POST /{apiV}/batch/surveyor/voting with a payload containing an array of 1 or more { surveyorId: '...', proof: '...' } elements, each corresponding to something created using a previous calls to GET /v2/surveyor/voting/{surveyorId}

@mrose17 mrose17 requested a review from evq May 11, 2018 00:08
@mrose17 mrose17 self-assigned this May 11, 2018
evq
evq previously requested changes May 11, 2018
Copy link
Contributor

@evq evq left a comment

Choose a reason for hiding this comment

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

can you add a batch vote to one of the end to end ledger tests? thanks!

@evq evq force-pushed the support-batched-votes branch 3 times, most recently from 117e0a3 to d62fd2c Compare May 15, 2018 22:27
@evq evq force-pushed the support-batched-votes branch from d62fd2c to 6464393 Compare May 15, 2018 22:41
@evq evq dismissed their stale review May 15, 2018 22:59

added the requested test

Copy link
Contributor

@evq evq left a comment

Choose a reason for hiding this comment

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

this looks good, tests are passing.

my only remaining request is that you relocate the surveyor.test.js that you added and restore the original test-unit command. (the test you added is an integration test, and there are actual unit tests for bat-utils) thanks!

mrose17 added 2 commits May 15, 2018 19:57
They pass when pointed at localhost running the branch
Copy link
Contributor

@evq evq left a comment

Choose a reason for hiding this comment

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

LGTM

@evq evq merged commit 4c7de52 into master May 16, 2018
@evq evq deleted the support-batched-votes branch May 16, 2018 02:17
@mrose17
Copy link
Contributor Author

mrose17 commented May 16, 2018

@maikelmclauflin - you ask

    this setup fails in travis. was the surveyor supposed to be updated to include actual values?

no. the "..." is used to test the parsing and error handling, there isn't an actual value in the tests.

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

Successfully merging this pull request may close these issues.

2 participants