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

Adding rinkeby support #2723

Merged
merged 18 commits into from Jul 10, 2017

Conversation

Projects
None yet
2 participants
@evertonfraga
Member

evertonfraga commented Jul 6, 2017

As a bonus, user can also choose a local private network.

evertonfraga added some commits Jul 6, 2017

@evertonfraga evertonfraga added this to the 0.9.0 milestone Jul 6, 2017

evertonfraga added some commits Jul 6, 2017

evertonfraga and others added some commits Jul 7, 2017

Alexandre Van de Sande
Alexandre Van de Sande

@evertonfraga evertonfraga self-assigned this Jul 7, 2017

@@ -46,7 +46,7 @@
</p>
{{#if TemplateVar.get "testnet"}}
<p>{{{i18n "mist.popupWindows.onboarding.mineItDescription"}}}</p>
<p>{{{i18n "mist.popupWindows.onboarding.FaucetDescription"}}}</p>

This comment has been minimized.

@frozeman

frozeman Jul 9, 2017

Member

I would have made that camelCase

This comment has been minimized.

@evertonfraga
@@ -81,7 +90,7 @@ Template['popupWindows_onboardingScreen'].helpers({
@method syncStatus
*/
'syncStatus': function () {
syncStatus() {

This comment has been minimized.

@frozeman

frozeman Jul 9, 2017

Member

Thats ES6 right here, if we would remove the ecmascript package, this would break.

else {
switch (network) {
// STARTS ROPSTEN
case 'test':

This comment has been minimized.

@frozeman

frozeman Jul 9, 2017

Member

why we call this "test" and the other by its name? To be backwards compatible?

This comment has been minimized.

@evertonfraga

evertonfraga Jul 9, 2017

Member

There are some reasons:

  • backwards compatibility, yes
  • Geth refers to the networks as --testnet and --rinkeby, so we're actually being consistent with their flags
  • forward compatibility: that'll work even if the testnet flag changes to a different chain, so we're not being specific to Ropsten in the internals, only when it's displayed.
@frozeman

This comment has been minimized.

Member

frozeman commented Jul 9, 2017

I didn't test it on my machine, but it looks good from the code changes.

@evertonfraga evertonfraga merged commit 0c64083 into develop Jul 10, 2017

1 of 3 checks passed

VersionEye There are all kind of security and license issues!
Details
codeclimate 3 new issues (39 fixed)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@evertonfraga evertonfraga deleted the add-rinkeby-support branch Jul 10, 2017

@lock

This comment has been minimized.

lock bot commented Mar 31, 2018

This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot locked and limited conversation to collaborators Mar 31, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.