On 18/08/18 19:30, Alejandro Piñeiro wrote:
On 18/08/18 06:36, Timothy Arceri wrote:
On 18/08/18 14:32, Timothy Arceri wrote:
Won't this cause shader runner to needlessly parse the .shader_test
file?
True, good point. In any case, as the test lacks a [test] section, full
shader.py runs would just check if the test links (assuming that
ARB_gl_spirv are available). Having said so ...
The file extension is also confusing. Maybe we should name these type
of files .shader_source or something similar rather than .shader_test ???
... this makes sense. That name fits better, and would avoid the
previous issue. As that would be a small change, I will make the change
locally, while I wait for the review of the other patches.
I've skimmed over most of the series and I didn't see anything other
than this that I had issue with. It's not very common to get a full
review for a piglit series. The general rule is once its been on the
list for a few weeks it's usually ok to push. After all it's better to
have wrong tests that can be fixed later than to have no tests at all.
I understand the external dependency might have stopped you from just
pushing but since its an optional dependency it's fine IMO.
Anyway for the series feel free to add:
Acked-by: Timothy Arceri <[email protected]>
_______________________________________________
Piglit mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/piglit
