Taylor Blau <me@xxxxxxxxxxxx> writes: > On Wed, Feb 10, 2021 at 12:36:19PM -0800, Junio C Hamano wrote: >> "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx> >> writes: >> >> > -test_expect_success 'setup: helpers for corruption tests' ' >> > - sha1_file() { >> > - remainder=${1#??} && >> > - firsttwo=${1%$remainder} && >> > - echo ".git/objects/$firsttwo/$remainder" >> > - } && >> > +sha1_file () { >> > + git rev-parse --git-path objects/$(test_oid_to_path "$1") >> > +} >> >> Yeah, back when 90cf590f (fsck: optionally show more helpful info >> for broken links, 2016-07-17) originally introduced this pattern, >> we didn't have nicely abstracted helper, but now we do, and there >> is no reason not to use it. Nice. > > This has nothing to do with this series, but I do notice a number of > other uses of test_oid_to_path that are doing this exact thing. In fact, > many of them don't use "git rev-parse --git-path", which would be > better. > > I wonder if it's worth a clean-up on top to consolidate all of those > "combine the loose object path of the object with xyz OID and the > $GIT_DIR/objects directory". > > In either case -- and I think I'm pretty clearly being pedantic at this > point -- do you suppose it's worthwhile to rename sha1_file to something > that doesn't have sha1 in it? Possibly. That is probably outside the scope of this topic, but we see such SHA -> HASH clean-up patches in different places, and this certainly is a fair game for such a clean-up, I would think. Thanks.