Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: > With your patch, there are now four users of the `is_clean` value, and > all but one of them ask for the negated value. Excellent observation. That strongly argues for the flipping of polarity, i.e. many people want to know "is it unclean/dirty?". It is funny that the name of the helper function where the value comes from, i.e. has_uncommitted_changes(), has the desired polarity. > It's not really the responsibility of this patch series, but I could > imagine that it would be nicer to future readers if a patch was added that > would invert the meaning of that variable and rename it to > `needs_committing`. At least to me, that would make the intention of the > code eminently clearer. While I agree, after reading the code, that it would make it easier to follow to flip the polarity of the variable, I would advise against renaming from state based naming (is it dirty?) to action based naming (must we commit?), *if* the variable is checked to sometimes see if we has something that we _could_ commit, while some other times to see if we _must_ commit before we can let the user proceed. "Does the index hold some changes to be committed?" is better question than "Must we commit?" or "Could we commit?" to derive the name of this variable from if that is the case, I would think.
