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

Change equality comparisons to == and != #135

Merged
merged 3 commits into from
Oct 19, 2016
Merged

Conversation

lilleyse
Copy link
Contributor

Related to this discussion: CesiumGS/cesium#4336 (comment)

If we decide to be loose about type comparisons in the JS implementation we may also want to change === to == to force casting. Right now a style with 5 < "6" would return true but 5 === "5" would be false.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 15, 2016

I think I understand that you want to add == and != so that they behave similar to < and friends with type coercion, but why are we removing the JavaScript best practice === and !== operators? Because they can't be implemented with the GLSL transpiler? If so, please explain.

@lilleyse
Copy link
Contributor Author

Oh I was thinking we would not support both == and === at the same time, but that's definitely doable.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 15, 2016

OK, let's do it, thanks.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 19, 2016

@lilleyse can you please update this PR and also update the styling spec to include changes needed for point clouds, e.g., pointSize in this PR or a new one?

@lilleyse
Copy link
Contributor Author

Updated. I'll open a new PR for the other styling and point cloud changes.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 19, 2016

Looks good.

@pjcozzi pjcozzi merged commit 7240f33 into master Oct 19, 2016
@pjcozzi pjcozzi deleted the make-equality-loose branch October 19, 2016 14:36
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.

2 participants