GSoC: Week 8
Week 8
Hello peeps!
This week, I was working on a patch to remove the global variable merge_log_config. As I said last week, I created a function in config.c to replace the usage of the variable. I got reviews from my mentor Christian, who said that I should move that function to fmt-merge-msg.c and since the merge_log_config variable is used to just set the value of the shortlog_len variable in cmd_merge() and cmd_fmt_merge_msg(), I should modify the function to return the value of the shortlog_len instead. I also removed the dependency on the_repository in builtin/fmt-merge-msg.c and wrote a test to make sure that git fmt-merge-msg -h can be called outside a repository. I sent the patch series to the mailing list: builtin/fmt-merge-msg: remove dependency on global variables and ‘the_repository’.
This changes the behaviour a bit which is addressed by Junio:
One obvious behaviour change I can see can happen when you have an invalid value set to merge.summary and run the command with command line override with the “–log” option. In the current code, the config callback barfs when it notices an invalid merge.summary setting, even though it won’t be used because the valid value given via the “–log” option would override it. In the updated code, adjust_shortlog_len() would short-circuit and does not even bother reading from the configuration, so the user will not be notified of a broken configuration.
So, I think we need to get more reviews to reach a consensus that this behaviour is okay.
I also received reviews from Junio regarding calling repo_config() after parse_options() as it is a bad idea to read the command-line options first and then reading values from the configuration. This might override the options given by the user in command-line. Although we might go ahead without facing any bugs for now, future developers might face issues when trying to add new options. So he asked me to go through how repo_config() is working without a repository and also look at some patches related to it where they allowed NULL repository to pass to it.
That’s it for this week. Thank you for reading my blog!
-Ayush:)