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

Improve add_attrs_if_missing() #155

Merged
merged 2 commits into from Aug 18, 2015

Conversation

@nox
Copy link
Member

nox commented Aug 18, 2015

Review on Reviewable

// FIXME: quadratic time
attrs.retain(|attr|
!existing.iter().any(|e| e.name == attr.name));
let names = attrs.iter().map(|e| e.name.clone()).collect::<HashSet<_>>();

This comment has been minimized.

@SimonSapin

SimonSapin Aug 18, 2015

Member

existing.iter(), right?

Maybe rename names to existing_names, and move the .retain() call to a .map() on the result of .into_iter()

This comment has been minimized.

@nox

nox Aug 18, 2015

Author Member

I fixed that, but forgot to commit before pushing… Will address the other points while at it.

@nox nox force-pushed the nox:quadratic-add-attrs-if-missing branch from cc7ff1b to dfd64ec Aug 18, 2015
@SimonSapin
Copy link
Member

SimonSapin commented Aug 18, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Aug 18, 2015

📌 Commit dfd64ec has been approved by SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented Aug 18, 2015

Testing commit dfd64ec with merge 3656fcd...

bors-servo pushed a commit that referenced this pull request Aug 18, 2015
Improve add_attrs_if_missing()



<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/html5ever/155)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 18, 2015

☀️ Test successful - travis

@bors-servo bors-servo merged commit dfd64ec into servo:master Aug 18, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request Aug 18, 2015
@nox nox deleted the nox:quadratic-add-attrs-if-missing branch Sep 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.