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

Display contract block creation #4069

Merged
merged 9 commits into from
Jan 9, 2017
Merged

Display contract block creation #4069

merged 9 commits into from
Jan 9, 2017

Conversation

ngotchac
Copy link
Contributor

@ngotchac ngotchac commented Jan 6, 2017

Closes #3173

The logic is : add to the meta the block number at which the contract has been deployed.
The best would be able to have a way to get the exact block number for any contract, but that would require to have access to internal transactions, which is for now only available for nodes running in trace mode... Another solution would be to make a call to Etherscan, but it is still in Beta right now (and isn't great anyway).

image

image

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Jan 6, 2017
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 14c2cc8 on ng-contract-block-creation into ** on master**.

@@ -139,6 +139,7 @@ export default class Contract {
}

setState({ state: 'hasReceipt', receipt });
this._receipt = receipt;
Copy link
Contributor

Choose a reason for hiding this comment

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

Anything with _ is private and should not be accessed outside of the module at all, i.e. the implementation can change completely and it should only affect the module, in this case the API.

So setting it here and accessing it in a modal is a no-go area.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. Fixed

@jacogr
Copy link
Contributor

jacogr commented Jan 6, 2017

One critical flaw on forcing external access to private denoted members, merge conflicts.

};

if (summary) {
return createReactElement(summary, props);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm really not convinced we should be going down this route with createElement, even if it is in a single place. Now there is magic is the caller, instead of being explicit and here we are doign what JSX should be doing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 9b5a5a3 on ng-contract-block-creation into ** on master**.

@jacogr jacogr added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 9, 2017
@jacogr jacogr merged commit cf0a20f into master Jan 9, 2017
@jacogr jacogr deleted the ng-contract-block-creation branch January 9, 2017 08:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants