Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> t/t9999-test.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++
If this is about the basic tree traversal logic, that belongs somewhere in
2xxx series. You seem to be testing only diff-tree and nothing else, so I
suspect that you shouldn't even consume a new test number but just adding
a new test or two to 4013 should suffice.
> +test_expect_success 'diff' '
> + cat >expected <<-\EOF &&
What's the point of the dash in "<<-\EOF" if you are not going to indent
the here doc?
> +:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M one/file
> +:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M one/two/file
> +:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M one/zoo
> +EOF
Outside t0xxx series please avoid making the test depend on absolute
object names like this. Since you are using only two blobs, you could
run two hash-objects to learn the hash for "" and "1\n" and say
cat >expected <<-EOF &&
:100644 100644 $none $one M one/file
...
EOF
Better yet, think if you even need the raw output here? Perhaps using
output formats like --name-only or --name-status is more appropriate and
make the test more readable?
> diff --git a/tree-walk.c b/tree-walk.c
> index a2f4a00..fd19681 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -532,7 +532,7 @@ int tree_entry_interesting(const struct name_entry *entry, const char *base, int
>
> pathlen = tree_entry_len(entry->path, entry->sha1);
>
> - for (i = 0; i < ps->nr; i++) {
> + for (i = ps->nr-1; i >= 0; i--) {
No explanation in the commit log message why. Besides, spell binary "-"
operator with a SP each on both ends, please.
Also the code seems to use a new term subpathspec without defining nor
explaining. Not good.
--
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