"Randall S. Becker" <rsbecker@xxxxxxxxxxxxx> writes: > Here are a few examples, there are more: > > auto_crlf = git_config_bool(var, value); > ^ The carets in your message do not align to what I think they are trying to point at, but I think the above is pointing at the '=' and wants to say "auto_crlf variable is enum, it gets assigned an integer returned from git_config_bool(), and I do not like that assignment". For this one I tend to agree with the compiler, meaning that it is ugly to define "enum auto_crlf" in such a way that the first two values happen to match what a logically different "enum" (which is "boolean") assigns the two values to. And this judgment does not change whether git_config_bool() actually returns an enum or an int (the code in reality returns the latter). I do not think people would terribly mind a patch to turn the above into: auto_crlf = git_config_bool(var, value) ? AUTO_CRLF_FALSE : AUTO_CRLF_TRUE; > "/home/jenkins/.jenkins/workspace/Build_Git/config.c", line 1147: > warning(272): > enumerated type mixed with another type > > type = sha1_object_info(s->oid.hash, &s->size); > ^ /* returns enum object_type or negative */ int sha1_object_info(const unsigned char *sha1, unsigned long *sizep) The function has been like this forever, I suspect, and I would say "this gives negative when error, or enum we know is non-negative" is quite a reasonable thing to do, but the enum has OBJ_BAD defined to be negative, so probably it is more kosher if sha1_object_info() is declared to return "enum object_type" instead of int. > "/home/jenkins/.jenkins/workspace/Build_Git/diff.c", line 3618: > warning(272): > enumerated type mixed with another type > > options->color_moved = diff_color_moved_default; > ^ If color_moved field is declared to be an enum, the _default variable should be, too. I do not think it is such a controversial fix.
