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

refactor(window): reference winit where applicable #20303

Merged
merged 1 commit into from Mar 15, 2018

Conversation

@kwonoj
Copy link
Contributor

kwonoj commented Mar 15, 2018

This is nearly first changes to rustlang code and I suspect I could make possible mistakes lot, please point me anything if need to be updated. 🙏

This PR intends to update codebase per #20299 , mostly followed suggestion around #20299 (comment) to update reference to use winit where applicable. Confirmed local build / check works correctly for updated references.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #20299 (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable

@kwonoj kwonoj force-pushed the kwonoj:refactor-winit branch from 8628d52 to aeec54a Mar 15, 2018
@SimonSapin
Copy link
Member

SimonSapin commented Mar 15, 2018

Looks good, thanks! There’s just a few more items that are in both glutin and winit and can be imported from the latter: ElementState, Event, MouseButton, MouseScrollDelta, VirtualKeyCode, TouchPhase, ActivationPolicy, WindowBuilderExt, MouseCursor, VirtualKeyCode (again)

… I think. I haven’t double-checked all of them, but the compiler should tell you. In general, winit deals with windows and input events while glutin adds OpenGL contexts/apis.

@kwonoj kwonoj force-pushed the kwonoj:refactor-winit branch from aeec54a to aa54cb7 Mar 15, 2018
@kwonoj
Copy link
Contributor Author

kwonoj commented Mar 15, 2018

@SimonSapin Appreciate for suggestion, I've added changes. Now other than {Api, GlContext, GlRequest} code uses winit directly where applicable.

@SimonSapin
Copy link
Member

SimonSapin commented Mar 15, 2018

Looks great!

@kwonoj
Copy link
Contributor Author

kwonoj commented Mar 15, 2018

uh, seems build failure on CI. let me try to amend it as well.

- relates with #20299
@kwonoj kwonoj force-pushed the kwonoj:refactor-winit branch from aa54cb7 to ecb465b Mar 15, 2018
@SimonSapin
Copy link
Member

SimonSapin commented Mar 15, 2018

Oops I meant to r+, but yeah there are build failures that need to be fixed before this can land. I don’t understand them based on reading the diff, though…

@kwonoj
Copy link
Contributor Author

kwonoj commented Mar 15, 2018

@SimonSapin I think it's my fault to not correctly move #cfg target for mac specific. I've pushed new changes and checking CI results now 🤞

@SimonSapin
Copy link
Member

SimonSapin commented Mar 15, 2018

Oh that makes sense. I think we don’t need to wait for Travis.

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Mar 15, 2018

📌 Commit ecb465b has been approved by SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented Mar 15, 2018

Testing commit ecb465b with merge 5f40842...

bors-servo added a commit that referenced this pull request Mar 15, 2018
refactor(window): reference winit where applicable

<!-- Please describe your changes on the following line: -->
This is nearly first changes to rustlang code and I suspect I could make possible mistakes lot, please point me anything if need to be updated. 🙏

This PR intends to update codebase per #20299 , mostly followed suggestion around #20299 (comment) to update reference to use `winit` where applicable. Confirmed local build / check works correctly for updated references.

---
<!-- 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 #20299 (github issue number if applicable).

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

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- 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="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20303)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Mar 15, 2018

@bors-servo bors-servo merged commit ecb465b into servo:master Mar 15, 2018
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request Mar 15, 2018
3 of 5 tasks complete
@kwonoj kwonoj deleted the kwonoj:refactor-winit branch Mar 15, 2018
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.

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