On Mon, 3 Feb 2025 at 12:16, Patrick Steinhardt <ps@xxxxxx> wrote: > > On Sat, Feb 01, 2025 at 12:12:00PM +0530, Meet Soni wrote: > > Rename `query_refspecs()` to `find_refspec_match` for clarity, as it > > finds a single matching refspec. > > > > Rename `query_refspecs_multiple()` to `find_all_refspec_matches` to > > better reflect that it collects all matching refspecs instead of > > returning just the first match. > > > > Rename `query_matches_negative_refspec()` to > > `find_negative_refspec_match` for consistency with the updated naming > > convention. > > Okay. The message might've read a tiny bit easier if it was a bulleted > list of renames. E.g.: > > We're about to move a couple of functions related to handling of > refspecs from "remote.c" into "refspec.c". In preparation for this > move, rename them to better reflect their intent: > > - `query_refspecs()` becomes `find_refspec_match()` for clarity, > as it finds a single matching refspec. > > ... Makes sense. > > I was wondering a bit about why we rename the static functions, as we > wouldn't have to expose them in a subsequent step anyway. Other than > that I think we should adhere to our coding guidelines with the renamed > public functions: Since we were renaming the query_* functions that are exposed, I updated the static ones as well to maintain naming consistency across related functions. > > The primary data structure that a subsystem 'S' deals with is called > `struct S`. Functions that operate on `struct S` are named > `S_<verb>()` and should generally receive a pointer to `struct S` as > first parameter. E.g. > > So: > > - `query_refspecs()` would be renamed to `refspec_find_match()`. > > - `query_refspecs_multiple()` would be renamed to > `refspec_find_all_matches()`. > > - `find_negative_refspec_match()` would be renamed to > `refspec_find_negative_match()`. > > Patrick Thanks for the review. Meet
