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:)
