I once made a PR changing a= f(b) from a="" if b =="" else f(b). About six months later still in pending review. I get that people have their own shit but damn.
Yeah, the two aren’t equivalent and the original has more conditions than the new one, so without context this just doesn’t make sense in this example.
A is “” only when B is also “”, otherwise we return f()
In the new one we simply say that regardless of what B is, we’ll just call f(), entirely skipping the case where B == “”.
Probably this specific condition checking was moved to the inner scope of f(), but this example does not tell us (who don’t know the context) that. Or maybe the check is redundant, but that also isn’t signaled in any way.
Or then maybe I’m just oblivious to the optimization, in which case I can see why the maintainer would take their time figuring that out. It’s not anything obvious based on that alone, at least to me, and I would say I have some experience in this field.
Edit: But yeah this is basically just semantics, I’m sure they gave apt description in the PR, so the context would be explained there and none of this really matters. I just like to ruminate about little things like this for some reason. Didn’t mean to imply they didn’t do a good PR, just that this specific example was either confusing or confused.
I once made a PR changing
a = f(b)froma = "" if b == "" else f(b). About six months later still inpending review. I get that people have their own shit but damn.edit) Sorry. Clarified the code.
It seems like you are implying this is an obvious optimization, regardless of context. Why do we not care what’s going on with
b?Yeah, the two aren’t equivalent and the original has more conditions than the new one, so without context this just doesn’t make sense in this example.
A is “” only when B is also “”, otherwise we return f()
In the new one we simply say that regardless of what B is, we’ll just call f(), entirely skipping the case where B == “”.
Probably this specific condition checking was moved to the inner scope of f(), but this example does not tell us (who don’t know the context) that. Or maybe the check is redundant, but that also isn’t signaled in any way.
Or then maybe I’m just oblivious to the optimization, in which case I can see why the maintainer would take their time figuring that out. It’s not anything obvious based on that alone, at least to me, and I would say I have some experience in this field.
Edit: But yeah this is basically just semantics, I’m sure they gave apt description in the PR, so the context would be explained there and none of this really matters. I just like to ruminate about little things like this for some reason. Didn’t mean to imply they didn’t do a good PR, just that this specific example was either confusing or confused.
I’ve missed the
f(b)part. Edited.