On Sat, Feb 01, 2020 at 11:03:37AM +0100, Torsten Bögershausen wrote:
> > diff --git a/sha1-file.c b/sha1-file.c
> > index 13b90b1cb1..e789bfd153 100644
> > --- a/sha1-file.c
> > +++ b/sha1-file.c
> > @@ -1647,7 +1647,8 @@ void *read_object_with_reference(struct repository *r,
> > }
> > }
> >
> > -static void write_object_file_prepare(const void *buf, unsigned long len,
> > +static void write_object_file_prepare(const struct git_hash_algo *algo,
> > + const void *buf, unsigned long len,
> > const char *type, struct object_id *oid,
> > char *hdr, int *hdrlen)
> > {
>
> One minor minor suggestion/nit, may be my own type of style only.
>
> When adding a parameter to a function, we typically add it at the end of
> the parameter list, rather than at the beginning.
> The other (unwritten) convention, when dealing with "buffer pointer/len",
> seams to be that buffer-ptr is the first paramter and len is is the second.
I'd usually add new options at the end, too, all things being equal. But
in this case, I see:
- some of these are input parameters and some are output (I think oid,
hdr, and hdrlen are the latter); it makes sense to keep them grouped
that way
- in object-oriented functions, we'd often take the object first
(e.g., strbuf_add). In non-object-oriented functions, we often take
a "context" argument first that shapes how the function works (e.g.,
all of the pp_* functions that take a pretty_print_context). I'd
think of the algorithm a little bit as a "context" argument, though
I admit it's mostly a gut feeling and quite subjective.
But if you believe that line of reasoning, then it doesn't seem out
of place as the first argument.
-Peff
