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

chore: update clr formula #5494

Merged
merged 2 commits into from Nov 20, 2019
Merged

chore: update clr formula #5494

merged 2 commits into from Nov 20, 2019

Conversation

thelostone-mc
Copy link
Member

@thelostone-mc thelostone-mc commented Nov 13, 2019

Based on the changes made by @frankchen07 after discussing with vitalik

@frankchen07 I haven't commented out the one off contribution -> let me know if that is something you would still want to remove. Cause in that case if a grant has no contributors -> it'll appear in the UI that contributing any amount would have a matching 0 as pair could not be formed

ref: thelostone-mc/clr@ef292e4

fixes: #5425

@codecov
Copy link

codecov bot commented Nov 13, 2019

Codecov Report

Merging #5494 into master will increase coverage by 0.08%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5494      +/-   ##
==========================================
+ Coverage   29.13%   29.21%   +0.08%     
==========================================
  Files         242      244       +2     
  Lines       20908    20962      +54     
  Branches     3022     3027       +5     
==========================================
+ Hits         6091     6124      +33     
- Misses      14569    14593      +24     
+ Partials      248      245       -3
Impacted Files Coverage Δ
app/grants/clr.py 0% <0%> (ø) ⬆️
app/kudos/admin.py 58.82% <0%> (-2.91%) ⬇️
app/grants/views.py 15.9% <0%> (-0.42%) ⬇️
app/retail/emails.py 23.25% <0%> (ø) ⬆️
...rketing/management/commands/no_applicants_email.py 0% <0%> (ø) ⬆️
app/chat/apps.py 0% <0%> (ø)
app/chat/views.py 57.14% <0%> (ø)
app/dashboard/views.py 12.71% <0%> (+0.01%) ⬆️
app/app/settings.py 79.71% <0%> (+0.07%) ⬆️
app/retail/views.py 27.67% <0%> (+0.08%) ⬆️
... and 4 more

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 38a08e7...4f7a095. Read the comment docs.

@frankchen07
Copy link
Contributor

frankchen07 commented Nov 18, 2019

@thelostone-mc - I think for the sake of making an accurate QV statement, if a grant only has 1 donator, the CLR match will be 0, since that's how we're performing pairwise matching.

The downside risk here is the "potential match" is marketing as well, just tough to say how effective it was compared to blasting it out on twitter and getting Vitalik's help.

I say we trend towards being true and accurate to QV in the front as we are in the behind the scenes calculations, and if we want to later add something saying "get a friend to donate too and you'll get a match!" we can salvage the marketing situation.

Copy link
Contributor

@frankchen07 frankchen07 left a comment

Choose a reason for hiding this comment

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

LGTM - just make sure we comment out the snippet of code to ensure if no pair = no contribution.

@thelostone-mc
Copy link
Member Author

@frankchen07 done ^_^

@thelostone-mc thelostone-mc merged commit 30e1ef4 into gitcoinco:master Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants