From: David Michael Barr Date: Mon, 07 Jun 2010 13:36:21 GMT Subject: Re: [PATCH 5/6] Add infrastructure to write revisions in fast-export format Message-ID: In-Reply-To: Hi Ram, >> Style: would probably be clearer to write: >> >> while (~(name = *path++)) { >> dirent = repo_dirent_by_name(dir, name); >> if (!dirent || !repo_dirent_is_dir(dirent)) >> break; >> dir = repo_dir_from_dirent(dirent); >> } >> >> i.e., fewer unnecessary braces, and dealing with the exceptional cases >> separately from the normal case. > > This was (probably unintentionally) re-factored by David when merging > in his `dirents` branch. Oh no, it was intentional. >> My 80-column terminal is suffering. Why not use the common >> pattern? > > Re-factored (again, probably unintentionally) by David during the merge. As above, totally intentional. @Jonathan: Thanks for the review, I took it into account when refactoring dirents. >> These limits are not checked; is the caller supposed to check them >> itself? Does svn obey them? > > I asked David too, and as far as we know, these limits are pretty > arbitrary. They're no enforced by SVN or any specific filesystem. We > can discuss about having less arbitrary limits and checking it in > svndump.c (while parsing the dump). I am indeed in favour of less arbitrary limits. The current magic numbers were chosen as common practical bounds.