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

Switch android to using glutin. Remove GLUT port. #4115

Closed
wants to merge 1 commit into from

Conversation

glennw
Copy link
Member

@glennw glennw commented Nov 27, 2014

NOTE: This shouldn't be merged until servo/android-rs-glue#1 has been merged.

@hoppipolla-critic-bot
Copy link

Critic review: https://critic.hoppipolla.co.uk/r/3294

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@glennw
Copy link
Member Author

glennw commented Nov 27, 2014

r? @mbrubeck and/or @larsbergstrom

@@ -1,4 +1,3 @@
/.cargo/config
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to not do this and have .cargo/config be in a subdirectory, like in #4099?

Copy link
Member Author

Choose a reason for hiding this comment

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

It could possibly go in ports/glutin - I'll test that on android this morning and update the PR.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately that doesn't get picked up by cargo, since android is now built like all other projects from the root of the source tree :(

Copy link
Member

Choose a reason for hiding this comment

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

I think #4140 could help with that. Whichever lands first, I can help rebasing the other one.

@jdm jdm added S-awaiting-review There is new code that needs to be reviewed. S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 28, 2014
bors-servo pushed a commit that referenced this pull request Dec 2, 2014
bors-servo pushed a commit that referenced this pull request Dec 2, 2014
bors-servo pushed a commit that referenced this pull request Dec 3, 2014
bors-servo pushed a commit that referenced this pull request Dec 3, 2014
bors-servo pushed a commit that referenced this pull request Dec 3, 2014
bors-servo pushed a commit that referenced this pull request Dec 3, 2014
@glennw
Copy link
Member Author

glennw commented Dec 8, 2014

I'm going to close this PR and re-open a new one after rebasing - too much has changed during the last week or so with android + ssl etc.

@glennw glennw closed this Dec 8, 2014
@SimonSapin
Copy link
Member

As mentioned in Critic comments, please consider making android-rs-glue a Cargo dependency, or if that doesn’t work a git-submodule. Having mach manually clone and build it seems like reinventing mechanisms that already exist.

@glennw glennw deleted the android-glutin branch February 11, 2015 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-needs-code-changes Changes have not yet been made that were requested by a reviewer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants