Re: [PATCH 2/2] index-pack: --fsck-objects to take an optional argument for fsck msgs
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 25, 2024, 23:13 UTC
- Message-ID
- <xmqqplxpropf.fsf@gitster.g>
- In-Reply-To
- <074e0c7ab923777c66516ced18b4fd1dadf7677f.1706215884.git.gitgitgadget@gmail.com>
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: John Cai <johncai86@gmail.com> > > git-index-pack has a --strict mode that can take an optional argument to
"mode" -> "option", probably.
Show 6 quoted lines
> provide a list of fsck issues to change their severity. --fsck-objects > does not have such a utility, which would be useful if one would like to > be more lenient or strict on data integrity in a repository. > > Like --strict, Allow --fsck-objects to also take a list of fsck msgs to > change the severity.
"Allow" -> "allow".
> This commit also removes the "For internal use only" note for > --fsck-objects, and documents the option. This won't often be used by > the normal end user, but it turns out it is useful for Git forges like > GitLab.
"This commit also removes", "documents" -> "Remove", "document".
Show 5 quoted lines
> ---fsck-objects:: > - For internal use only. > +--fsck-objects[=<msg-ids>]:: > + Instructs index-pack to check for broken objects instead of broken > + links. If `<msg-ids>` is passed, it should be a comma-separated list of
Very much pleased to see an additional description that is written to clarify the difference between this option and the other --strict option. The original was totally unclear on this point, and it is very much appreciated.
The other option notices and dies upon seeing either a broken object or a dangling link. This one only diagnoses broken objects and does not care if the objects are connected. However, saying "instead of" here tempts readers to mistakenly think that the other one only checks links and this one only checks contents, which is not what we want to say. Perhaps "to check for broken objects, but unlike `--strict`, do not choke on broken links" or something?
Same comment on <msg-ids> as the previous step.
> + `<msg-id>=<severity>` where `<msg-id>` and `<severity>` are used to > + change the severity of `fsck` errors, eg: `--strict="missingEmail=ignore,badTagName=ignore"`.
Same comment for "eg:" as before.
Show 11 quoted lines
> @@ -1785,8 +1785,9 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
> } else if (!strcmp(arg, "--check-self-contained-and-connected")) {
> strict = 1;
> check_self_contained_and_connected = 1;
> - } else if (!strcmp(arg, "--fsck-objects")) {
> + } else if (skip_to_optional_arg(arg, "--fsck-objects", &arg)) {
> do_fsck_object = 1;
> + fsck_set_msg_types(&fsck_options, arg);
> } else if (!strcmp(arg, "--verify")) {
> verify = 1;
> } else if (!strcmp(arg, "--verify-stat")) {The implementation of this part looks quite obvious, once you see how "--strict[=<msgid>=<level>]" is implemented.
Looking good.
Thanks.