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

Add profiling for getters and setters #11

Merged
merged 4 commits into from
May 11, 2017
Merged

Add profiling for getters and setters #11

merged 4 commits into from
May 11, 2017

Conversation

RiftLurker
Copy link
Contributor

Builds on top of #10 as that PR kind of breaks profiling of getters.

This will only add profiling to configurable getters and setters, this has to be set true explicitly.

See more at Object.defineProperty.

ES6 getter-Functions are invoked, even when called from prototype.
This fix uses Object.getOwnPropertyDescriptor and checks if the get or set property is present.
@gdborton
Copy link
Collaborator

Seems legit to me, my subscription has lapsed, but I want to check this out before pulling it in. Should I close the other pr in favor of this one?

@RiftLurker
Copy link
Contributor Author

I've taken care of the other PR, thank you for having a look at it.

@RiftLurker
Copy link
Contributor Author

@gdborton any progress on this pull request?

@gdborton
Copy link
Collaborator

Seems legit. I know this has been pending for a long time, and that you've got a fork running. Is there anything that you need to update before merging?

@RiftLurker
Copy link
Contributor Author

I think this is in a fine state right now and ready to get merged.

@gdborton gdborton merged commit 7c45be2 into screepers:master May 11, 2017
@RiftLurker RiftLurker deleted the getset-profiler branch December 22, 2017 14:09
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.

2 participants