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

Remove CompositorEventListener trait? #11339

Closed
Ms2ger opened this issue May 23, 2016 · 4 comments
Closed

Remove CompositorEventListener trait? #11339

Ms2ger opened this issue May 23, 2016 · 4 comments

Comments

@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented May 23, 2016

Only IOCompositor implements it now, and I doubt we'll reintroduce other implementations.

@pcwalton
Copy link
Contributor

@pcwalton pcwalton commented May 24, 2016

Yeah, feel free to do this if it's feasible.

@kyleheadley
Copy link
Contributor

@kyleheadley kyleheadley commented May 24, 2016

I can take this, I need to learn about the PR system anyway. It sounds like this is just to reorganize? I'll move the trait code into the struct impl and test.

cc @asajeffrey

@Ms2ger
Copy link
Contributor Author

@Ms2ger Ms2ger commented May 25, 2016

Though I notice there's one advantage: it erases the Window type parameter.

@kyleheadley kyleheadley mentioned this issue May 26, 2016
4 of 5 tasks complete
bors-servo added a commit that referenced this issue May 27, 2016
Remove CompositorEventListener trait

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #11339 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because refactoring

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11443)
<!-- Reviewable:end -->
@kyleheadley
Copy link
Contributor

@kyleheadley kyleheadley commented May 27, 2016

I attempted to remove the trait object all together, but the cef port (at least) depends on the dynamic dispatch. I'm looking into altering cef, but that may be beyond the scope of this issue.

bors-servo added a commit that referenced this issue May 27, 2016
Remove CompositorEventListener trait

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #11339 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because refactoring

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11443)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Jun 1, 2016
Remove CompositorEventListener trait

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #11339 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because refactoring

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11443)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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