Re: [PATCH] environment: move access to "core.attributesfile" into repo settings

[Ayush Chandekar <ayu.chandekar@gmail.com>]

Hey,
thanks for reviewing the patch!

> We typically switch the order around a bit in our commit messages: we
> first explain what the actual problem is, and then we say how we fix it.

Got it.


> Hm. I wonder what the actual merit of this function is after the
> refactoring. Right now there isn't really any as it is a direct wrapper
> of `repo_settings_get_attributesfile_path()`.

I can remove the function and replace all the instances with
`repo_settings_get_attributesfile_path()`. What do you think?

> I think it would make sense to split out this change into a separate
> commit. The first commit would move the config into "repo-settings.c",
> the second commit would adapt functions and their callers as necessary.

Alright.

> Extraneous newline.
Apologies. Will fix it.

> I think we should just retain `git_attr_val_global()` and plug in
> `the_repository`. The extra change here doesn't add anything, and
> "builtin/var.c" being a builtin means is not reused anywhere else,
> either.

Makes sense. I will drop `repo_git_attr_val_globa()` and keep
`git_attr_val_global()` with `the_repository`.


> We don't use curly braces around one-line statements.
Will fix it.

> One thing I'm missing is the code to `free()` the allocated memory in
> `repo_settings_clear()`.
Oh right.

> > \ No newline at end of file

Got it.

Thanks,
Ayush:)