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

Development #83

Merged
merged 22 commits into from
Feb 27, 2019
Merged

Development #83

merged 22 commits into from
Feb 27, 2019

Conversation

zennermac
Copy link
Contributor

Incremental Update, explorer urls, zencashjs update to 1.2.0 and bip39 seed generation update to generate 12 word seeds.
Various comment cleanups.

Jake Tarren and others added 21 commits October 9, 2018 18:41
Bump version number on about page, and update description in `package.json`
BrandExpansion! And Cordova Updates.
Copy link
Contributor

@vherus vherus left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, just a couple of minor quality improvements

src/containers/SendPage.js Outdated Show resolved Hide resolved
insightAPI: 'https://explorer.zensystem.io/insight-api-zen/',
explorerURL: 'https://explorer.zensystem.io/',
insightAPI: 'https://explorer.horizen.global/api/',
explorerURL: 'https://explorer.horizen.global/',
Copy link
Contributor

Choose a reason for hiding this comment

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

I can see this URL dotted around in different places as a hard string, it would be much better moved to a constant like LANG_ENGLISH and CURRENCY_USD so we only need to update it in one place if it changes again

</ListItem>
<ListItem tappable onClick={() => this.props.setInsightAPI('http://explorer.zenmine.pro/insight-api-zen/')}>
zenmine.pro
<ListItem tappable onClick={() => this.props.setInsightAPI('https://explorer.horizen.global/api/')}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace with constant as detailed below

Copy link
Contributor

@mauriziobinello mauriziobinello left a comment

Choose a reason for hiding this comment

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

all ok

@zennermac zennermac merged commit 41818bc into master Feb 27, 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
Development

Successfully merging this pull request may close these issues.

5 participants