Re: [PATCH v3 0/5] cleanup duplicate name_compare() functions
Jonathan, On Wed, Jun 18, 2014 at 12:14:07PM -0700, Jonathan Nieder wrote: Jeremiah Mahler wrote: Jeremiah Mahler (5): cache: rename cache_name_compare() to name_compare() tree-walk.c: remove name_compare() function unpack-trees.c: remove name_compare() function dir.c: rename to name_compare() name-hash.c: rename to name_compare() cache.h| 2 +- dir.c | 3 +-- name-hash.c| 2 +- read-cache.c | 23 +-- tree-walk.c| 10 -- unpack-trees.c | 11 --- 6 files changed, 16 insertions(+), 35 deletions(-) After looking at the patches I suspect this should be a single patch. That way it's bisectable, and the changes outside of read-cache.c are small enough that it's not too much of a burden to review as a single patch. That would be a pain to bisect if the partial application of the patch set left the system in a broken state. Good suggestion. The code change looked good. Thanks and hope that helps, Jonathan Thanks, -- Jeremiah Mahler jmmah...@gmail.com http://github.com/jmahler -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/5] cleanup duplicate name_compare() functions
On Thu, Jun 19, 2014 at 01:04:32AM -0700, Jeremiah Mahler wrote: After looking at the patches I suspect this should be a single patch. That way it's bisectable, and the changes outside of read-cache.c are small enough that it's not too much of a burden to review as a single patch. That would be a pain to bisect if the partial application of the patch set left the system in a broken state. Good suggestion. One trick I use, especially when refactoring, is to use an interactive rebase to test each commit in isolation, like: GIT_EDITOR='sed -i /^pick .*/aexec make -j8 test' git rebase -i After picking each commit, that will run the tests on each one[1]. If it fails, the rebase will pause. You can fix any problems, test to your satisfaction, commit --amend, and then rebase --continue to keep going. -Peff [1] Of course it can be rather time-consuming for a large series. I often just compile-test at first, and then do a final make test pass when I think everything is right. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/5] cleanup duplicate name_compare() functions
Version 3 of the patch series to cleanup duplicate name_compare() functions (previously was 'add strnncmp() function' [1]). This version goes in a slightly different direction than the previous version. Before I was trying to add a strnncmp() function so I could remove duplicate copies of the name_compare() function in tree-walk.c and unpack-trees.c. But then Torsten Bögershausen pointed out that there is a cache_name_compare() function which is nearly identical to name_compare() [2]*. * cache_name_compare() is not identical to name_compare(). The former returns +1, -1, whereas the latter returns +N, -N. But there is no place where name_compare() was used that needed the magnitude so this change would not alter its behavior. So I decided why not generalize the name of cache_name_compare() by renaming it to name_compare(), since it doesn't do anything with caches, other than being part of cache.h and read-cache.c. Then the duplicate name_compare() functions can be removed and the few places that used cache_name_compare() can be renamed to name_compare(). It cleans up the code with a minimal number of changes. It keeps existing functions instead of creating new ones. And there are several other functions in cache.h that are similarly named '*name_compare' so it follows the already established style. Also, the name_compare() now uses memcmp() as it did originally instead of using strncmp() as it did in the last version. [1]: http://marc.info/?l=gitm=140299051431479w=2 [2]: http://marc.info/?l=gitm=140300329403706w=2 Jeremiah Mahler (5): cache: rename cache_name_compare() to name_compare() tree-walk.c: remove name_compare() function unpack-trees.c: remove name_compare() function dir.c: rename to name_compare() name-hash.c: rename to name_compare() cache.h| 2 +- dir.c | 3 +-- name-hash.c| 2 +- read-cache.c | 23 +-- tree-walk.c| 10 -- unpack-trees.c | 11 --- 6 files changed, 16 insertions(+), 35 deletions(-) -- 2.0.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/5] cleanup duplicate name_compare() functions
Jeremiah Mahler wrote: Jeremiah Mahler (5): cache: rename cache_name_compare() to name_compare() tree-walk.c: remove name_compare() function unpack-trees.c: remove name_compare() function dir.c: rename to name_compare() name-hash.c: rename to name_compare() cache.h| 2 +- dir.c | 3 +-- name-hash.c| 2 +- read-cache.c | 23 +-- tree-walk.c| 10 -- unpack-trees.c | 11 --- 6 files changed, 16 insertions(+), 35 deletions(-) After looking at the patches I suspect this should be a single patch. That way it's bisectable, and the changes outside of read-cache.c are small enough that it's not too much of a burden to review as a single patch. The code change looked good. Thanks and hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
