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

Add gl::gl_type() #105

Merged
merged 1 commit into from Jan 27, 2017
Merged

Add gl::gl_type() #105

merged 1 commit into from Jan 27, 2017

Conversation

sotaroikeda
Copy link
Contributor

@sotaroikeda sotaroikeda commented Jan 23, 2017

Fixes #104.


This change is Reviewable

@sotaroikeda
Copy link
Contributor Author

r? @kvark

@kvark
Copy link
Member

kvark commented Jan 25, 2017

I don't think gl_type method itself is needed. The client can just do GlType directly or call Default::default() when the type is expected.
@sotaroikeda do you mind removing the gl_type method?

@sotaroikeda
Copy link
Contributor Author

@kvark , I am thinking to use gl_type () for #99. How can we detect current gl type without gl_type ()?

@sotaroikeda
Copy link
Contributor Author

@kvark , if you have another idea to detect current gl context type for dynamic gl type selection(#99), can you give me your idea?

@emilio
Copy link
Member

emilio commented Jan 27, 2017

How is this decided dynamically at all with this patch? Or is this preliminar work only?

@sotaroikeda
Copy link
Contributor Author

This preliminar work only. I created #104, since this function could exit on both static and dynamic gl type selection.
WIP patch is in https://bugzilla.mozilla.org/show_bug.cgi?id=1325911.

@kvark
Copy link
Member

kvark commented Jan 27, 2017

@sotaroikeda sorry, this PR slipped past me...
You don't need gl_type() function itself. Where GlType is expected, one can just do Default::default().

@sotaroikeda
Copy link
Contributor Author

Sorry, I did not get your point well. Ok, I removed gl_type(). @kvark, can you review again?

@kvark
Copy link
Member

kvark commented Jan 27, 2017

Thanks!
@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 25ef544 has been approved by kvark

@bors-servo
Copy link
Contributor

⌛ Testing commit 25ef544 with merge 8d1c49b...

bors-servo pushed a commit that referenced this pull request Jan 27, 2017
Add gl::gl_type()

Fixes #104.

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

☀️ Test successful - status-travis

@bors-servo bors-servo merged commit 25ef544 into servo:master Jan 27, 2017
@kvark
Copy link
Member

kvark commented Jan 27, 2017

@sotaroikeda gleam-v0.2.32 is published now: https://crates.io/crates/gleam/0.2.32

@sotaroikeda sotaroikeda deleted the Bug1325911 branch March 10, 2017 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants