hadronized / glsl

GLSL parser for Rust
190 stars 28 forks source link

Move tests to parse_tests #115

Closed jrmuizel closed 4 years ago

jrmuizel commented 4 years ago

This avoids having to parse the tests during a non test build and keeps parsers.rs under the line length limit for import into mozilla-central.

hadronized commented 4 years ago

Hey! I don’t understand that change. Why is it not enough with the #[cfg(test)] in the parsers.rs file?

jrmuizel commented 4 years ago

[cfg(test)] stuff will still need to be parsed. I believe the

#[cfg(test)]
mod parse_tests;

should prevent the entire file from being read.

That being said the bigger motivation is that Firefox has a lint that checks third-party vendored crates for large files to avoid accidentally including them. Splitting this out lets us avoid the hassle of working around that.

hadronized commented 4 years ago

That’s a weird use case. Also, you mentioned mozilla-central, but I’ve never been told glsl was making its way into moz-central. Do you have some links / discussions / context about that topic, please? :)

jrmuizel commented 4 years ago

Sure. We're using it for the WebRender CPU fallback. We translate the shaders to C++ using https://github.com/jrmuizel/glsl-to-cxx which depends on glsl and then run them using https://github.com/jrmuizel/sw-wr/

hadronized commented 4 years ago

Cool! I find it a bit weird of a change. But I’ve always wanted to move unit tests about module A from A.rs into… something else. Isn’t there a way that wouldn’t require to add the pub everywhere? That could accidentally leak a private function / type.

jrmuizel commented 4 years ago

I made those functions pub(crate) instead.

jrmuizel commented 4 years ago

So do you think this change will work?

hadronized commented 4 years ago

Yep, sure! I’ll release them tomorrow! :)

hadronized commented 4 years ago

Released as part of glsl-4.0.3.