On Sun, Oct 1, 2023 at 9:22 PM Eric W. Biederman <ebiederm@xxxxxxxxx> wrote:
> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
> > On Wed, Sep 27, 2023 at 3:55 PM Eric W. Biederman <ebiederm@xxxxxxxxx> wrote:
> >> + die(_("Failed to convert object from %s to %s"),
> >> + from->name, to->name);
> >
> > s/Failed/failed/
>
> I don't understand wanting to start a sentence with a lower case letter.
> Can you explain?
Consistency with most of the rest of the messages emitted by Git.
CodingGuidelines call for lowercase and omission of the full-stop
(period).
The choice, of course, is subjective, but these are the guidelines
upon which the project has settled for better or worse. (You can
certainly find older code, which predates the guideline, using
capitalized messages, but it's good to adhere to the guideline for new
code if possible.)
> >> +int repo_oid_to_algop(struct repository *repo, const struct object_id *src,
> >> + const struct git_hash_algo *to, struct object_id *dest);
> >
> > I suppose the function name is pretty much self-explanatory to those
> > familiar with the underlying concepts, but it might still be helpful
> > to add a comment explaining what the function does.
>
> I could use words that repeat what is in the function signature.
> But I don't think I could add anything.
>
> I would have to say something like:
>
> Look up the oid that an equivalent object would have in a repository
> whose object format is "to".
>
> Is that helpful?
That would help clarify the function a bit for me. Explaining the
function's return value could also be useful.
