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.

Sign up for GitHub

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

Jump to bottom

Use external 'threadpool' crate, remove in-tree utility. #9788

Merged
merged 1 commit into from
Mar 19, 2016
Merged

Use external 'threadpool' crate, remove in-tree utility. #9788

merged 1 commit into from
Mar 19, 2016

Conversation

frewsxcv
Copy link
Contributor

Review on Reviewable

@highfive
Copy link

warning Warning warning

  • These commits modify net code, but no tests are modified. Please consider adding a test!

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 28, 2016
@frewsxcv
Copy link
Contributor Author

If anyone wants to see a direct comparison with the new ThreadPool:

https://github.com/frewsxcv/rust-threadpool/blob/master/src/lib.rs

@frewsxcv frewsxcv closed this Mar 1, 2016
@frewsxcv frewsxcv deleted the threadpool branch March 1, 2016 16:54
@frewsxcv
Copy link
Contributor Author

frewsxcv commented Mar 1, 2016 

11:12 AM <•Ms2ger> I'd be more in favour if the repo lived in the servo org and you bumped to 1.0

I don't like bumping for the sake of bumping so I'm going to pass.

@frewsxcv frewsxcv restored the threadpool branch March 6, 2016 17:19
@frewsxcv frewsxcv reopened this Mar 6, 2016
@frewsxcv
Copy link
Contributor Author

frewsxcv commented Mar 6, 2016

I don't like bumping for the sake of bumping so I'm going to pass.

After thinking about it for a few days, I'm reversing my stance on this.

rust-threadpool/rust-threadpool#22

@Ms2ger
Copy link
Contributor

Ms2ger commented Mar 7, 2016

Also interested in moving this into servo/? I don't like reducing bus factors more than necessary.

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #9892) made this pull request unmergeable. Please resolve the merge conflicts.

@highfive highfive added the S-needs-rebase There are merge conflict errors. label Mar 7, 2016
@frewsxcv frewsxcv removed the S-needs-rebase There are merge conflict errors. label Mar 7, 2016
@frewsxcv
Copy link
Contributor Author

frewsxcv commented Mar 9, 2016

I don't like reducing bus factors more than necessary.

Would it be okay if I added Servo people as "Collaborators" to the repository?

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #10055) made this pull request unmergeable. Please resolve the merge conflicts.

@highfive highfive added the S-needs-rebase There are merge conflict errors. label Mar 17, 2016
@frewsxcv frewsxcv removed the S-needs-rebase There are merge conflict errors. label Mar 17, 2016
@frewsxcv
Copy link
Contributor Author

Merge conflicts addressed.

@frewsxcv
Copy link
Contributor Author

I added Ms2ger, SimonSapin, nox, and Manishearth as collaborators to the repository. I also gave the 'publish' Servo org team publish access on crates.io.

@Ms2ger
Copy link
Contributor

Ms2ger commented Mar 17, 2016

That's three times UTC+1/2, and Manish. I'd like to have Australia/US coverage too.

@frewsxcv
Copy link
Contributor Author

Just added larsberg.

EDIT: (I'm also US)

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #10070) made this pull request unmergeable. Please resolve the merge conflicts.

@highfive highfive added the S-needs-rebase There are merge conflict errors. label Mar 18, 2016
@jdm
Copy link
Member

jdm commented Mar 18, 2016

I'll r+ a rebase.

@bors-servo
Copy link
Contributor

📌 Commit 8c265c7 has been approved by jdm

@bors-servo
Copy link
Contributor

⌛ Testing commit 8c265c7 with merge 0a0992e...

@highfive highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Mar 18, 2016
bors-servo pushed a commit that referenced this pull request Mar 18, 2016
Auto merge of #9788 - servo:threadpool, r=jdm
0a0992e 
Use external 'threadpool' crate, remove in-tree utility.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9788)
<!-- Reviewable:end -->
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-needs-rebase There are merge conflict errors. labels Mar 18, 2016
@jdm
Copy link
Member

jdm commented Mar 18, 2016

@bors-servo: r-
This is missing updates to the other lock files, unfortunately.

@frewsxcv
Copy link
Contributor Author

This is missing updates to the other lock files, unfortunately.

How do I update the other lockfiles?

@bors-servo
Copy link
Contributor

💔 Test failed - gonk

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Mar 18, 2016
@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Mar 18, 2016
@frewsxcv
Copy link
Contributor Author

Updated other lockfiles.

@jdm
Copy link
Member

jdm commented Mar 18, 2016

@bors-servo: r+

@bors-servo
Copy link
Contributor

📌 Commit 182b9b1 has been approved by jdm

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Mar 18, 2016
bors-servo pushed a commit that referenced this pull request Mar 19, 2016
Auto merge of #9788 - servo:threadpool, r=jdm
9e0c73c 
Use external 'threadpool' crate, remove in-tree utility.

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

⌛ Testing commit 182b9b1 with merge 9e0c73c...

@bors-servo
Copy link
Contributor

💔 Test failed - gonk

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Mar 19, 2016
@KiChjang
Copy link
Contributor

@bors-servo retry

  • The annoying git

@bors-servo
Copy link
Contributor

⚡ Previous build results for linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, status-appveyor are reusable. Rebuilding only android, gonk...

@bors-servo
Copy link
Contributor

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, status-appveyor

@bors-servo bors-servo merged commit 182b9b1 into master Mar 19, 2016
@frewsxcv frewsxcv deleted the threadpool branch March 19, 2016 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@pcwalton pcwalton

Labels
S-tests-failed The changes caused existing tests to fail.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@frewsxcv @highfive @Ms2ger @bors-servo @jdm @KiChjang @pcwalton