-
Notifications
You must be signed in to change notification settings - Fork 193
Conversation
971ced3
to
6099be5
Compare
CI currently fails on master with the same error as well. |
Why are the serializations changing? Why do we have all these new |
It was changed in v2. Now human-readable formats use a string, which contain the name of the flags. JSON is a human-readable format, that's why it's affecting the tests. In See https://github.com/bitflags/bitflags/blob/2.0.0/CHANGELOG.md#changes-to-serde-serialization.
In v1 the This was done in a PR like this bitflags/bitflags#282 and is unfortunately not highlighted in the changelog. |
@daxpedda I fixed the failing CI on master, could you rebase? |
Done. |
This error doesn't make a whole lot of sense to me, my PR should have no effect on this. But somehow CI on master succeeds, so what gives? --- a/tests/out/glsl/force_point_size_vertex_shader_webgl.fs_main.Fragment.glsl
+++ b/tests/out/glsl/force_point_size_vertex_shader_webgl.fs_main.Fragment.glsl
@@ -1,4 +1,5 @@
#version 300 es
+#extension GL_EXT_texture_shadow_lod : require
precision highp float;
precision highp int; Should I just fix it? |
b08ed0e
to
e5c7cc8
Compare
I'm not sure how much value there is in deriving |
The only value I can think of for these (and |
We should derive only the traits we actually need. I believe they contribute to compile time, even if you don't use them, but more importantly, deriving them leads one to wonder what those traits are used for. Seeing that a trait is not derived is actually a helpful hint (of course, there could be impls elsewhere, so it's just a hint, but...). |
Some of these types are public and they previously had all of those traits automatically derived by bitflags. I imagine we always want |
Proposal: remove |
Done. |
Some types require |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Changelog: https://github.com/bitflags/bitflags/blob/2.0.0/CHANGELOG.md#200.
This is a breaking change. The API that is implemented by the macro is now different.
I was a bit liberal with deriving
std
traits on public types, which mimics the behavior ofbitflags
v1, let me know if we don't need all these traits. Potentially we could at least dropPartialOrd
andOrd
.