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

Store Option<ImageInfo> instead of making fields of ImageInfo optional #24582

Closed
jdm opened this issue Oct 29, 2019 · 3 comments
Closed

Store Option<ImageInfo> instead of making fields of ImageInfo optional #24582

jdm opened this issue Oct 29, 2019 · 3 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Oct 29, 2019

The code in WebGLTexture stores an array of ImageInfo objects. Each of these objects has a is_initialized field, and other fields that are given default values. This makes the code harder to reason about; it would be easier to read if the array contained Option<ImageInfo> instead and any checks for initialization checked for is_none() or is_some() instead.

Code: components/script/dom/webgltexture.rs
Tests: ./mach test-wpt tests/wpt/webgl/tests/conformance/textures (there should be no difference before and after)

@highfive
Copy link

@highfive highfive commented Oct 29, 2019

Hi! If you have any questions regarding this issue, feel free to make a comment here, or ask it in the #servo channel in IRC.

If you intend to work on this issue, then add @highfive: assign me to your comment, and I'll assign this to you. 😄

@teapotd
Copy link
Contributor

@teapotd teapotd commented Oct 29, 2019

@highfive assign me

@highfive highfive added the C-assigned label Oct 29, 2019
@highfive
Copy link

@highfive highfive commented Oct 29, 2019

Hey @teapotd! Thanks for your interest in working on this issue. It's now assigned to you!

bors-servo added a commit that referenced this issue Oct 31, 2019
Store Option<ImageInfo> instead of making fields of ImageInfo optional

Fixes #24582

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #24582
- [X] These changes do not require tests
bors-servo added a commit that referenced this issue Nov 1, 2019
Store Option<ImageInfo> instead of making fields of ImageInfo optional

Fixes #24582

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #24582
- [X] These changes do not require tests
bors-servo added a commit that referenced this issue Nov 1, 2019
Store Option<ImageInfo> instead of making fields of ImageInfo optional

Fixes #24582

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #24582
- [X] These changes do not require tests
bors-servo added a commit that referenced this issue Nov 1, 2019
Store Option<ImageInfo> instead of making fields of ImageInfo optional

Fixes #24582

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #24582
- [X] These changes do not require tests
bors-servo added a commit that referenced this issue Nov 1, 2019
Store Option<ImageInfo> instead of making fields of ImageInfo optional

Fixes #24582

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #24582
- [X] These changes do not require tests
@bors-servo bors-servo closed this in 56537fa Nov 2, 2019
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.

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