substrait-io / substrait

A cross platform way to express data transformation, relational algebra, standardized record expression and plans.
https://substrait.io
Apache License 2.0
1.14k stars 148 forks source link

refactor: prefer boolean over BOOLEAN #590

Closed westonpace closed 7 months ago

westonpace commented 7 months ago

There were some instances of BOOLEAN and some instances of boolean. All other human readable type names in the YAML are using lowercase (except when we use templated names like T). The docs state that we prefer lowercase:

https://github.com/substrait-io/substrait/blob/3251b1fc5ede5788502be989b8eab778051d7a4d/site/docs/types/type_parsing.md?plain=1#L13

westonpace commented 7 months ago

This isn't entirely random :laughing: :

https://github.com/westonpace/substrait-expr/blob/2df9f77918bbee44d11343fb106aa215ee62aa84/substrait-expr-funcgen/src/lib.rs#L42-L44

westonpace commented 7 months ago

Funny you kept the quotes ;)

I have a YAML auto-formatter that I use for my own stuff. I have to disable it for Substrait because it ends up rewriting everything. Maybe I'll try and setup a CI job to apply it.

In the meantime, I'm trying to restrict my changes to things that actually parse differently (though not always successfully)

vbarua commented 7 months ago

Maybe I'll try and setup a CI job to apply it.

+1 to auto-formatting this and enforcing in CI.