dir.c: fix comments to agree with argument name
PATCH 1/1 dir.c: fix comments to agree with argument name
From: Alex Vandiver <[email protected]>
---
dir.c: Fix comments to agree with argument name
Comments are out of date with the variable names.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-757%2Fnipunn1313%2Fcomments-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-757/nipunn1313/comments-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/757
dir.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
dir.c +3 -3
@@ -1040,9 +1040,9 @@ static int add_patterns_from_buffer(char *buf, size_t size,* an index if 'istate' is non-null), parse it and store the * exclude rules in "pl". * -* If "ss" is not NULL, compute SHA-1 of the exclude file and fill +* If "oid_stat" is not NULL, compute SHA-1 of the exclude file and fill * stat data from disk (only valid if add_patterns returns zero). If -* ss_valid is non-zero, "ss" must contain good value as input. +* oid_stat.valid is non-zero, "oid_stat" must contain good value as input. */ static int add_patterns(const char *fname, const char *base, int baselen, struct pattern_list *pl, struct index_state *istate,
@@ -1090,7 +1090,7 @@ static int add_patterns(const char *fname, const char *base, int baselen,int pos;if (oid_stat->valid && !match_stat_data_racy (istate, &oid_stat->stat, &st)) - ;/* no content change, ss->sha1 still good */ + ;/* no content change, oid_stat->oid still good */ else if (istate && (pos = index_name_pos(istate, fname, strlen(fname))) >= 0 && !ce_stage(istate->cache[pos]) &&
base-commit: d4a392452e292ff924e79ec8458611c0f679d6d4
Signature · 1 line
gitgitgadget
dir.c static int add_patterns_from_buffer(char *buf, size_t size, · line 1043
Quoted 10 lines · Nipunn Koorapati via GitGitGadget
dir.c +1
-1@@ -1040,9 +1040,9 @@ static int add_patterns_from_buffer(char *buf, size_t size,* an index if 'istate' is non-null), parse it and store the * exclude rules in "pl". * -* If "ss" is not NULL, compute SHA-1 of the exclude file and fill +* If "oid_stat" is not NULL, compute SHA-1 of the exclude file and fill
Makes sense. This changed as part of 4b33e60201 (dir: convert struct sha1_stat to use object_id, 2018-01-28). Perhaps it would likewise make sense to stop saying "SHA-1" here, and just say "hash" (or even "object id", though TBH I think the fact that the hash is the same as an object-id is largely an implementation detail).
-Peff
PATCH v2 1/1 dir.c: fix comments to agree with argument name
In reply to Nipunn Koorapati via GitGitGadgetFrom: Alex Vandiver <[email protected]>
---
dir.c: Fix comments to agree with argument name
Comments are out of date with the variable names.
Update since v1
* Change "SHA1" to "oid" for consistency as suggested by Peff
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-757%2Fnipunn1313%2Fcomments-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-757/nipunn1313/comments-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/757
Range-diff vs v1:
1: 08ad8fe7af ! 1: 571fa7dd3d dir.c: fix comments to agree with argument name
@@ dir.c: static int add_patterns_from_buffer(char *buf, size_t size,
* exclude rules in "pl".
*
- * If "ss" is not NULL, compute SHA-1 of the exclude file and fill
-+ * If "oid_stat" is not NULL, compute SHA-1 of the exclude file and fill ++ * If "oid_stat" is not NULL, compute oid of the exclude file and fill
* stat data from disk (only valid if add_patterns returns zero). If
- * ss_valid is non-zero, "ss" must contain good value as input.
+ * oid_stat.valid is non-zero, "oid_stat" must contain good value as input.
dir.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
dir.c +3 -3
@@ -1040,9 +1040,9 @@ static int add_patterns_from_buffer(char *buf, size_t size,* an index if 'istate' is non-null), parse it and store the * exclude rules in "pl". * -* If "ss" is not NULL, compute SHA-1 of the exclude file and fill +* If "oid_stat" is not NULL, compute oid of the exclude file and fill * stat data from disk (only valid if add_patterns returns zero). If -* ss_valid is non-zero, "ss" must contain good value as input. +* oid_stat.valid is non-zero, "oid_stat" must contain good value as input. */ static int add_patterns(const char *fname, const char *base, int baselen, struct pattern_list *pl, struct index_state *istate,
@@ -1090,7 +1090,7 @@ static int add_patterns(const char *fname, const char *base, int baselen,int pos;if (oid_stat->valid && !match_stat_data_racy (istate, &oid_stat->stat, &st)) - ;/* no content change, ss->sha1 still good */ + ;/* no content change, oid_stat->oid still good */ else if (istate && (pos = index_name_pos(istate, fname, strlen(fname))) >= 0 && !ce_stage(istate->cache[pos]) &&
base-commit: d4a392452e292ff924e79ec8458611c0f679d6d4
Signature · 1 line
gitgitgadget
Quoted 8 lines · Jeff King
Quoted 2 lines
- * If "ss" is not NULL, compute SHA-1 of the exclude file and fill
+ * If "oid_stat" is not NULL, compute SHA-1 of the exclude file and fillMakes sense. This changed as part of
4b33e60201(dir: convert structsha1_statto useobject_id, 2018-01-28). Perhaps it would likewise make sense to stop saying "SHA-1" here, and just say "hash" (or even "object id", though TBH I think the fact that the hash is the same as an object-id is largely an implementation detail).
I do not quite get your "though TBH", though. I do agree with you that it is an implementation detail that an object is named after the hash of its contents, so saying "compute object name" probably makes sense in more context than "compute hash" outside the narrow parts of the code that actually implements how object names are computed. So I would have expected "because TBH", not "though TBH".
Anyway. Nipunn, can you fix both of them in the same commit, as they are addressing a problem from the same cause (i.e. we are no longer SHA-1 centric).
Thanks.
Happy to update it to use the object based terminology, though I'm not sure how the desired final result differs from above.
I believe I said "compute oid" in the comment - and it is all in one commit.
gitgitgadget appears to have shown a range-diff from the previous iteration, but the latest iteration is still one commit.
--Nipunn
Quoted 24 lines · Junio C Hamano
Quoted 8 lines · Jeff King
Quoted 2 lines
- * If "ss" is not NULL, compute SHA-1 of the exclude file and fill
+ * If "oid_stat" is not NULL, compute SHA-1 of the exclude file and fillMakes sense. This changed as part of
4b33e60201(dir: convert structsha1_statto useobject_id, 2018-01-28). Perhaps it would likewise make sense to stop saying "SHA-1" here, and just say "hash" (or even "object id", though TBH I think the fact that the hash is the same as an object-id is largely an implementation detail).I do not quite get your "though TBH", though. I do agree with you that it is an implementation detail that an object is named after the hash of its contents, so saying "compute object name" probably makes sense in more context than "compute hash" outside the narrow parts of the code that actually implements how object names are computed. So I would have expected "because TBH", not "though TBH".
Anyway. Nipunn, can you fix both of them in the same commit, as they are addressing a problem from the same cause (i.e. we are no longer SHA-1 centric).
Thanks.
Quoted 11 lines · Junio C Hamano
Quoted 8 lines · Jeff King
Quoted 2 lines
- * If "ss" is not NULL, compute SHA-1 of the exclude file and fill
+ * If "oid_stat" is not NULL, compute SHA-1 of the exclude file and fillMakes sense. This changed as part of
4b33e60201(dir: convert structsha1_statto useobject_id, 2018-01-28). Perhaps it would likewise make sense to stop saying "SHA-1" here, and just say "hash" (or even "object id", though TBH I think the fact that the hash is the same as an object-id is largely an implementation detail).
Quoted 6 lines
I do not quite get your "though TBH", though. I do agree with you that it is an implementation detail that an object is named after the hash of its contents, so saying "compute object name" probably makes sense in more context than "compute hash" outside the narrow parts of the code that actually implements how object names are computed. So I would have expected "because TBH", not "though TBH".
Sorry, I was just confused.
The implementation detail I meant is that we are using a "struct object_id" in the oid_stat type (and also that "oid_stat" is likewise exposing too much). I thought this was another version of our stat_validity, where we are checking quickly to see if a random file that is not necessarily part of the working tree has been updated.
And indeed, we do use it that way for files like .git/info/exclude, where having an "object_id" is really irrelevant. But we also use it for checking the validity of tracked files, where we populate it from an actual blob (see dir.c:do_read_blob).
So it is actually reasonable to expose that in the name, and to think about it as an object_id.
Quoted 3 lines
Anyway. Nipunn, can you fix both of them in the same commit, as they are addressing a problem from the same cause (i.e. we are no longer SHA-1 centric).
The v2 that Nipunn sent with "oid" in the comment looks good to me.
-Peff
Quoted 5 lines · Jeff King
Quoted 3 lines
Anyway. Nipunn, can you fix both of them in the same commit, as they are addressing a problem from the same cause (i.e. we are no longer SHA-1 centric).
The v2 that Nipunn sent with "oid" in the comment looks good to me.
OK, then all is good. Thanks.
Sounds good to me. Please let me know if there's anything further I need to do. I'm relatively new to git contributions - and my understanding is that at this point a maintainer would merge the commit at their leisure. Happy to do any further cleanup required to make that possible
Thanks
--Nipunn
Quoted 10 lines · Junio C Hamano
Quoted 5 lines · Jeff King
Quoted 3 lines
Anyway. Nipunn, can you fix both of them in the same commit, as they are addressing a problem from the same cause (i.e. we are no longer SHA-1 centric).
The v2 that Nipunn sent with "oid" in the comment looks good to me.
OK, then all is good. Thanks.