dai.js icon indicating copy to clipboard operation
dai.js copied to clipboard

return a default options obj with tally when no MKR support

Open b-pmcg opened this issue 4 years ago • 4 comments

b-pmcg avatar Nov 30 '21 00:11 b-pmcg

On the fence about this, if there's not MKR support on the options, do we really want to return a default, or should it just be handled on the front end? This PR makes an assumption that there are always 3 options in a plurality poll. Yes, its a fair assumption, but I'm conflicted about enforcing that in the code. @rafinskipg @tyler17 thoughts?

b-pmcg avatar Nov 30 '21 00:11 b-pmcg

Do you think we should add a default state for ranked polls too? Returning the current options as all "0"?

rafinskipg avatar Nov 30 '21 11:11 rafinskipg

I think it's a fair assumption, and looks like it shouldn't break if there does happen to be a plurality vote with more than the 0,1, and 2 options, right?

edit: actually, long ago, before rank choice was added, we had plurality polls with more than those 3 options, so we could test this update on those polls

maybe the best thing to do would be to somehow get the # of options a poll has and return that many default options. I don't think we have that data available in the current function though.

Do you think we should add a default state for ranked polls too? Returning the current options as all "0"?

Yeah we probably should do that, but like above, we'd probably want to retrieve the # of options a poll has and return that many defaults. I'll try to see what the most painless way to get a polls # options is, I don't think we want to do another fetch in this call just to get that info.

b-pmcg avatar Nov 30 '21 15:11 b-pmcg

Codecov Report

Merging #313 (9b3964c) into dev (b821dc8) will decrease coverage by 0.07%. The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #313      +/-   ##
==========================================
- Coverage   86.20%   86.13%   -0.08%     
==========================================
  Files         139      139              
  Lines        6337     6339       +2     
  Branches     1257     1257              
==========================================
- Hits         5463     5460       -3     
- Misses        869      874       +5     
  Partials        5        5              
Impacted Files Coverage Δ
...ges/dai-plugin-governance/src/GovPollingService.ts 84.82% <85.71%> (-0.32%) :arrow_down:
packages/dai/src/eth/Web3Service.ts 83.21% <0.00%> (-2.19%) :arrow_down:
packages/dai/src/eth/accounts/setup.js 74.07% <0.00%> (-1.24%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b821dc8...9b3964c. Read the comment docs.

codecov[bot] avatar Dec 03 '21 00:12 codecov[bot]