From: Pushkar Singh Date: Sun, 11 Jan 2026 11:01:39 GMT Subject: Re: [PATCH 1/2] t5403:introduce check_post_checkout helper function Message-ID: In-Reply-To: I agree with Eric’s point about documenting the helper’s arguments. Since patch 2 also changes the hook output format to a structured "old=… new=… flag=…" layout that "check_post_checkout()" depends on, it would be especially helpful if the function comment spelled out both the meaning of the four parameters and the expected on-disk format of the args file. That would make the helper’s contract much clearer to future readers and reduce the risk of accidental breakage if the hook output changes. On Sun, Jan 11, 2026 at 1:23 PM Eric Sunshine wrote: > > On Sun, Jan 11, 2026 at 2:30 AM Deveshi Dwivedi > wrote: > > The test file repeatedly uses the same four-line pattern to validate > > post-checkout hook arguments: read the args file, then test each of > > the three values individually. > > > > Introduce a check_post_checkout helper function that encapsulates this > > pattern. This patch does not change test behavior; it prepares the > > code for improvement in the next step. > > > > Signed-off-by: Deveshi Dwivedi > > --- > > diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh > > @@ -9,6 +9,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME > > +# Helper function to check post-checkout hook arguments > > +check_post_checkout () { > > + test "$#" = 4 || BUG "check_post_checkout takes 4 args" > > + read old new flag <"$1" && > > + test "$old" = "$2" && test "$new" = "$3" && test "$flag" = "$4" > > +} > > Rather than forcing people to read the function body to divine the > purpose of the four arguments, the function comment should spell out > their meaning. See the many "Usage:" comments in > t/test-lib-functions.sh for examples of how to write more useful > function documentation. >