search

SaschaWillems / Vulkan

C++ examples for the Vulkan graphics API
MIT License
10.34k stars 2.03k forks source link

Remove trailing whitespace in shaders directory #1134

Closed mmoult closed 5 months ago

mmoult commented 5 months ago

Great repo by the way! Thanks for making it.

When browsing the shader examples, I saw a lot of trailing whitespace. I figured I would remove it all and offer the change upstream if you want it.

SaschaWillems commented 5 months ago

Thanks for your PR, but if you do PRs that touch so many files (esp. if it's just clean up) it's always a good idea to ask first. Not gonna merge this for the time being.

mmoult commented 5 months ago

May I ask why you closed the request? You say "for the time being", which I interpret to mean you may want to merge it in the future. Should I reopen the request in, for example, six months? If you may want the change later, why not leave the request open?

I see trailing whitespace as irrelevant here, and all other things being equal, smaller file sizes are better than larger. If possible, I would like to know why you don't want to merge this (maybe the shaders shouldn't be changed, maybe you want to keep the git log clean from whitespace changes, etc). In other words, you must be seeing something nontrivial here that I would like to understand too, if you are free to share.

SaschaWillems commented 5 months ago

I'm not a native speaker, so my wording might not always be 100% correct. Please excuse this.

As for the reasons, aside from this being my personal spare time project: I don't have any requirements regarding code styles, which in turn means I don't care for trailing whitespaces.

So this PR would mostly be a temporal thing, as future changes to shaders or additional shaders might again contain trailing whitespaces. I have refused similar PRs in the past.

Hence why I asked to get in touch for PRs that touch a large no. of files (esp. with trivial changes) first.