Remove attribs that only reflect requested values by toji · Pull Request #574 · immersive-web/webxr · GitHub
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 attribs that only reflect requested values #574

Merged
merged 2 commits into from
Apr 19, 2019
Merged

Conversation

toji
Copy link
Member

@toji toji commented Mar 28, 2019

Fixes #513
Addresses some discussion in #514, but doesn't change the enums.

This PR removes the mode attrib from XRSession, the depth, stencil, and alpha attribs from XRWebGLLayer, and the subtype attrib from XRStationaryReferenceSpace. Since that left XRStationaryReferenceSpace with nothing else in it, I also took the opportunity to consolidate it and XRUnboundedReferenceSpace into XRReferenceSpace.

@toji
Copy link
Member Author

toji commented Apr 5, 2019

Nell requested that I split out the attribs and reference space bits of this PR, which I'm fine with. I'll get to that sometime in the next week or so.

@toji toji changed the title Remove attribs that only reflect requested values and condense reference space interfaces Remove attribs that only reflect requested values Apr 12, 2019
@toji
Copy link
Member Author

toji commented Apr 12, 2019

Separated changes as requested. Reference space consolidation is now handled in #587.

Copy link
Member

@NellWaliczek NellWaliczek left a comment

Choose a reason for hiding this comment

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

Minor feedback... I'm generally supportive of this change

explainer.md Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
@toji
Copy link
Member Author

toji commented Apr 16, 2019

Feedback addressed. Take one more look?

@toji toji added the potential breaking change Issues that may affect the core design structure. label Apr 17, 2019
@toji
Copy link
Member Author

toji commented Apr 18, 2019

Rebased. Review ping, @NellWaliczek?

Copy link
Member

@NellWaliczek NellWaliczek left a comment

Choose a reason for hiding this comment

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

Let's make sure the final commit title reflects which types we actually cleaned up, but other than that LGTM!

@toji toji merged commit 0841dbb into master Apr 19, 2019
@toji toji deleted the remove-echoes branch April 30, 2019 16:59
@AdaRoseCannon AdaRoseCannon added ed:explainer Include in newsletter, explainer change ed:spec Include in newsletter, spec change labels May 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ed:explainer Include in newsletter, explainer change ed:spec Include in newsletter, spec change potential breaking change Issues that may affect the core design structure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove unnecessary interface members that only reflect what was requested
3 participants