bug-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: update: patch: relative path bugs in cvs (client/server mode only)


From: Chris Bohn
Subject: Re: update: patch: relative path bugs in cvs (client/server mode only)
Date: Fri, 02 Apr 2004 11:30:10 -0500
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040113

regenerated against the head, attached to issue 81, and pasted below
(additionally, I fixed a hard-coded path in sanity.sh to be
${CVSROOT_DIRNAME}):

Doesn't look like it.  You may have hardcoded your own path and then
fixed that.

You are correct: in my earlier patch, I had accidently put a hard-coded path, which I fixed in the last patch.

Log entry:
2004-03-30        Chris Bohn  <address@hidden>

 * client.c, client.h (send_files, send_file_names, send_max_dotdot):

This is a bit more verbose than really necessary for a ChangeLog.
Please be more abstract in future ChangeLogs.  Describe fixes in more
detail in the emails containing the patch, in the issue, or in the
comments in the code when appropriate.

OK; sorry about that. I thought the information needed to be somewhere, and I happened to pick this spot.

I took out the call in send_file_names.  It is probably unecessary.  The
test suite tests a lot of cases, and if we couldn't find a broken one
yet, then I am content to wait for the bug report so we can fix it and
add a test.

I prefer to error on the side of caution because other times cvs has changed with little concern for backward compatibility, and it has bitten me (e.g., expansion of the loginfo string to use double quotes instead of single quotes; it broke my ability to use my one loginfo script with both cvs servers before and after this change). I do understand one needs to weigh the chance of some case breaking vs. keeping the code cleaned up.

Your patch was extremely mangled, with tabs converted to spaces and who
knows what else.  I read it and reimplemented it, mostly.

This was only my second patch to cvs, and the first was just a week or so ago. Since going through the process, I've wondered why you take patches as diffs in text files. That seems more complicated than needed and seems like more work for those who integrate the patch into the main trunk. Why not accept patches as checked-in source on a branch of the file? Tabs and all formatting will then certainly be maintained, and you can diff the files against whatever versions are most useful for you. You don't have to be looking at context and/or line numbers to try and piece together where in a particular source file parts of a patch belong. To me, it sounds like an easier way of getting at least the source code changes. You could use tags to identify the patch, etc. for easy reference.

Index: client.h
+/* Calculate and send max-dotdot to the server */
+void
+send_max_dotdot(int argc, char **argv);

I removed this prototype - this function is only called from within
client.c and doesn't need to export a prototype.

No problem; I thought the convention was to prototype everything in the header file. This might be a good addition to the HACKING file if convention is to only put a prototype in a header if it is used outside of that source. Looking back at the source, I realize there are lots of prototypes just at the top of client.c, but I didn't notice that originally, so a line in HACKING mentioning this may save you the trouble the next time.

Thanks for integrating it into the trunk; I look forward to a day when my company can just download the latest version and use it (for the past 5 years, the company has been using a locally modified version because of different issues that we had to work around). The last two issues now appear ready to be in the next release.

Chris





reply via email to

[Prev in Thread] Current Thread [Next in Thread]