On Thu, Jan 29, 2009 at 2:45 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Imagine what you would do when you discover another request a random other > client sends that you would want to ignore just like you did for 'noop'. > Viewed in this light, your patch is a very short sighted one that has a > big negative impact on maintainability. Fair enough. I was trying to change the minimal amount that I could and still fix the breakage. Your patch is much better. Not to mention terser ;-) > A true no-brainer that has no negative impact would have been something > like the attached patch, that adds a method that does not do anything. > > Even then, between req_CATCHALL and req_EMPTY, I am not sure which one is > expected by the clients, without consulting to the protocol documentation > for cvs server/client communication. In the attached patch, I am guessing > from your patch that at least Tortoise does not expect any response to > it. I have consulted the CVS protocol documentation (found at http://www.wandisco.com/techpubs/cvs-protocol.pdf), which states the following about the "noop" command: "Response expected: yes. This request is a null command in the sense that it doesn't do anything, but merely (as with any other requests expecting a response) sends back any responses pertaining to pending errors, pending Notified responses, etc." So apparently a response *is* expected. I'm not really familiar enough with CVS or git-cvsserver to determine what that means it should do, but I suspect from perusing the code that req_EMPTY is the appropriate action. Moreover, I've moved on from using git-cvsserver myself, having instead convinced my Windows-using compatriots to use msysgit instead. So if you feel that this change is unwarranted, feel free to just drop it. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
