Hi Taylor and Patrick
On Mon, Oct 23, 2023, at 19:40, Taylor Blau wrote:
>> Nit: we don't know about the intent of the caller, so they may not want
>> to write to the file but only read it.
>
> I was going to suggest that we allow the caller to pass in the flags
> that they wish for git_global_config() to pass down to access(2), but
> was surprised to see that we always use R_OK.
>
> But thinking on it for a moment longer, I realized that we don't care
> about write-level permissions for the config, since we want to instead
> open $GIT_DIR/config.lock for writing, and then rename() it into place,
> meaning we only care about whether or not we have write permissions on
> $GIT_DIR itself.
>
> I think in the existing location of this code, the "if we should write"
> portion of the comment is premature, since we don't know for sure
> whether or not we are writing. So I'd be fine with leaving it as-is, but
> changing the comment seems easy enough to do...
>
>> > + * location; error out even if XDG_CONFIG_HOME
>> > + * is set and points at a sane location.
>> > + */
>> > + die(_("$HOME not set"));
>>
>> Is it sensible to `die()` here in this new function that behaves more
>> like a library function? I imagine it would be more sensible to indicate
>> the error to the user and let them handle it accordingly.
>
> Agreed.
>
> Thanks,
> Taylor
What do you guys think the signature of `git_global_config` should be?
