Dark Mode

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[CLEANUP beta] Abstract chainWatchers into an object.#11819

Merged
rwjblue merged 1 commit intomasterfrom
abstract-chainwatchers
Jul 24, 2015
Merged

[CLEANUP beta] Abstract chainWatchers into an object.#11819
rwjblue merged 1 commit intomasterfrom
abstract-chainwatchers

Conversation

Copy link
Contributor

krisselden commented Jul 20, 2015

Abstracting chainWatchers into an Object, cleans up some shape deopts, helps make stuff more clear. This is good to go, I haven't squashed yet just so it is easier for people to review updates.

Copy link
Member

mixonic commented Jul 20, 2015

@krisselden is green!

Copy link
Member

stefanpenner Jul 20, 2015

Choose a reason for hiding this comment

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

do we ever delete from this.nodes?

Copy link
Contributor Author

krisselden Jul 20, 2015

Choose a reason for hiding this comment

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

no, we just empty the array

Copy link
Member

stefanpenner Jul 20, 2015

Choose a reason for hiding this comment

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

sounds good.

Copy link
Contributor Author

krisselden Jul 20, 2015

Choose a reason for hiding this comment

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

@stefanpenner should it just search a single array or linked list of well shaped entries? like {key: 'foo', node: node} ?

Copy link
Member

stefanpenner Jul 20, 2015

Choose a reason for hiding this comment

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

array should be good, as we just do a sequential scan anyways.

Copy link
Member

stefanpenner commented Jul 21, 2015

@krisselden whats blocking this/

mmun reviewed Jul 23, 2015
Copy link
Member

mmun Jul 23, 2015

Choose a reason for hiding this comment

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

function Empty() {}
Empty.prototype = Object.create(null);
...
this.nodes = new Empty();

krisselden force-pushed the abstract-chainwatchers branch from 4c2cc1f to 95d3f3f Compare July 23, 2015 09:11
krisselden force-pushed the abstract-chainwatchers branch from 95d3f3f to 2f63cab Compare July 23, 2015 23:24
Copy link
Contributor Author

krisselden commented Jul 23, 2015

@stefanpenner rebased and squashed

rwjblue added a commit that referenced this pull request Jul 24, 2015
[CLEANUP beta] Abstract chainWatchers into an object.
rwjblue merged commit be1d622 into master Jul 24, 2015
rwjblue deleted the abstract-chainwatchers branch July 24, 2015 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants