Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

Referrals updates #61

Merged
merged 11 commits into from
Feb 20, 2017
Merged

Referrals updates #61

merged 11 commits into from
Feb 20, 2017

Conversation

dylanlott
Copy link
Contributor

Referrals updates

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 95.692% when pulling 2413a4e on dylanlott:referrals into 0399950 on Storj:master.

Copy link
Contributor

@bryanchriswhite bryanchriswhite left a comment

Choose a reason for hiding this comment

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

I'd like to understand a little bit more about how the referrals model is used/works but otherwise these seem to be reasonable changes

@phutchins
Copy link
Contributor

LGTM. Merging.

@phutchins phutchins merged commit 7092ae4 into storj-archived:master Feb 20, 2017
@braydonf
Copy link
Contributor

braydonf commented Feb 20, 2017

Is this not work-in-progress (e.g. not ready to merge) as with the WIP in the title? Looks like there is API/model change here that will need major bump, and code coverage decreased.

@phutchins
Copy link
Contributor

@dylanlott & @barbaraliau , can you please remove WIP from this MR and discuss with @braydonf the API change and where the version should land for this? I can re-publish (unpublish and publish) the version that is decided upon.

@dylanlott dylanlott changed the title WIP Referrals updates Referrals updates Feb 20, 2017
'recipient.email': recipientEmail
}).then((referral) => {
if (referral) {
referral.count += referral.count;
Copy link
Contributor

Choose a reason for hiding this comment

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

There may be a concurrency issue here if two processes do this at the same time, one of the count will go missing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants