Re: [9front] [PATCH] Fix assert macro to not break on commas

[sirjofri <sirjofri+ml-9front@sirjofri.de>]

Hello,

My 2 cents:

I think having that change is unnecessary, at least. It only makes the code look nicer a little bit, and not having it makes the programmer recognize the "bug" pretty early. I'm pretty sure the compiler will just complain and the programmer adds another set of parentheses, and that's it. Furthermore, it makes it clear that assert is a macro and not a function, leading to further care when developing applications (which is the job of a programmer anyways; having assert as is and dealing with extra parentheses and compiler errors is too small to cry for a change of how things are currently, in my humble opinion).

In addition to that, programmers should always be able to look at the functions they use. In this case it's obvious that assert is a macro. Furthermore, changing that macro makes 9front code that depends on that change incompatible with other 9 systems. Again, just a small thing that's easy to "fix", but it's incompatible.

Changing assert to a function is something I don't like to see. I know it's just a fancy way to add the point of error to the error output, but I'd like to have that. Bug reports suck, and most users don't know about crash dumps etc, they just paste the error and in that case I'd like to have that extra bit of information, in the rare cases I don't capture the error earlier with proper output for the user. Also, it's a common functionality that's well known across environments and also programming languages.

Last words that are important: It's not bad if a patch is rejected. Most of the time it's good, and also it's fine to do any changes in your local environment, or even distribute your own version of 9 with any changes you like. Gate keepers try their best to judge about the incoming patches to keep a minimum (or higher level) of quality. Of course it's also fine to argue, so keep up the good work on both sides. (Maybe I should've argued more about my factotum totp stuff, but I see the point: there's no application currently using it).

sirjofri