Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

fix verification stores #3864

Merged
merged 3 commits into from
Dec 15, 2016
Merged

fix verification stores #3864

merged 3 commits into from
Dec 15, 2016

Conversation

derhuerst
Copy link
Contributor

@gavofyork please check if it works.

@derhuerst derhuerst added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Dec 15, 2016
@@ -54,7 +54,7 @@ export default class EmailVerificationStore extends VerificationStore {
}

constructor (api, account, isTestnet) {
super(api, EmailVerificationABI, 'emailverification3', account, isTestnet);
super(api, EmailVerificationABI, 4, account, isTestnet);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I 100% like the magic numbers (would probably make more sense as a constant to at least specify intent, especially in 3 months from now)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fully agree, that's why i had changed it to string in the first place. constants is a good solution i think.

@jacogr jacogr added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 15, 2016
@gavofyork gavofyork merged commit 5fb3457 into master Dec 15, 2016
@gavofyork gavofyork deleted the jr-fix-verification branch December 15, 2016 20:13
@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 85.916% when pulling d2962fe on jr-fix-verification into 5939575 on master.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants