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

async properties #202

Merged
merged 3 commits into from
May 12, 2015
Merged

async properties #202

merged 3 commits into from
May 12, 2015

Conversation

debris
Copy link
Contributor

@debris debris commented May 12, 2015

added:

  • async way of getting properties

removed:

  • property setter (it was unused)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 95.12% when pulling cd0df61 on debris:async_properties into 9812b01 on ethereum:develop.

return RequestManager.getInstance().send({
method: this.setter,
params: [this.formatInput(value)]
Property.prototype.asyncGet = function (callback) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Though i would habe called it getAsync

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

@frozeman
Copy link
Contributor

I really like the changes, but i makes myself question the whole use of "properties". Why we don't plainly use:

// sync
var res = web3.eth.blockNumber();

// async
web3.eth.blockNumber(function(e,res) { ... })

You can still use the properties synchronously, but you need to add a (). It at least makes everything more unified. Also in terms of the docs.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 95.12% when pulling 5f9272c on debris:async_properties into 9812b01 on ethereum:develop.

@debris
Copy link
Contributor Author

debris commented May 12, 2015

but we will break another thing then :) I would prefer to do it later, after merging this pr and releasing new version :)

@frozeman
Copy link
Contributor

ok. lets do it later then. it will anyway be very much recommended to use only async, except you know what you're doing.
so we can later get "rid" of the pure property ones..

frozeman added a commit that referenced this pull request May 12, 2015
@frozeman frozeman merged commit fa8db32 into web3:develop May 12, 2015
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.

None yet

3 participants