Jeff Trawick wrote: > My gut instinct when I see something odd is that I'd like to know what > that was for.
First off, I am not in a position to tell you why it was done like that, that came from the original contributor, so I don't know why you were asking me. Although having looked at it it is pretty obvious why. The original author had added ap_log_error() into the code, and when they were done they removed them again, but forgot to remove the server_rec they had wired through that made ap_log_error() possible. And it's pretty obvious how it could be missed by me. The comment at the top clearly explained why the server_rec had been added. I expected to see ap_log_error() further down, but instead got distracted for a few hours by the fact the patch had completely rewritten the function and in its original form didn't fully work. And that was fine too, because you picked it up, creating the opportunity for it to be fixed. > Yeah, I'm pretty sure it is wrong and needs to be > removed, but I wondered why as well. I suppose I could have said "I > don't see any purpose for this, so delete." Just flag it to be looked at further, as in "you missed a spot". > What I think is that you are not comfortable having a civil conversation > about your commits. > > Would "Please delete this unnecessary code" have been better received > than explaining why I thought it was preferable to leave that minimal > debug support out? It would have, yes. Rhetorical questions come across as "how could you have missed that moron" even though that may not have been the original intention. > It's late, I'm tired and I'm off to bed. The style can be fixed > tomorrow. > > > It will be appreciated. Fixed in r821538. Regards, Graham --
smime.p7s
Description: S/MIME Cryptographic Signature
