Hi Junio, first of all: the improvements discussed here are already part of v6. On 2015-06-19 19:33, Junio C Hamano wrote: > Johannes Schindelin <johannes.schindelin@xxxxxx> writes: > >> I basically made up names on the go, based on the messages. >> >>> Some of the questionable groups are: >>> >>> BAD_DATE DATE_OVERFLOW >> >> I guess it should be BAD_DATE_OVERFLOW to be more consistent? > > I am not sure about "consistency", but surely a common prefix would > help readers to group things. But for this particular group, I was > wondering if singling out "integer overflow", "zero stuffed > timestamp", etc. into such a finer sub-errors of "you have a bad > timestamp" was beneficial. Well, someone thought it a good idea to print different error messages, and I took that as an indicator that there is merit in being able to distinguish these issues from one another. >>> BAD_TREE_SHA1 INVALID_OBJECT_SHA1 INVALID_TREE >>> >>> BAD_PARENT_SHA1 INVALID_OBJECT_SHA1 >> >> So how about s/INVALID_/BAD_/g? > > It is not just about distinction between INVAID and BAD. > > I was basically wondering what rule decides which one among > BAD_TREE_SHA1, INVALID_OBJECT_SHA1 and INVALID_TREE I would get when > I have a random non-hexdigit string in various places, e.g. after > 'tree ' in the object header of a commit object, after 'tag ' in a > tag object that says 'type tree', etc. To be honest, I think the IDs do not really matter as much as your comment makes it sound: the IDs purpose is solely to be able to configure the message type (read: whether to error out, warn or ignore the issue). The real information is in the actual message (and I did not change any message, therefore I could not make things worse than they are right now). Example: you would never read BAD_TREE_SHA1, but instead: "badtreesha1: invalid 'tree' line format - bad sha1". For BAD_OBJECT_SHA1 (as it is now called), there are actually two code paths generating the error: "invalid 'object' line format - bad sha1" and "no valid object to fsck". And for BAD_TREE it is: "could not load commit's tree <SHA1>". Thus, from the error message it should be really clear what is going on. >>> Also it is unclear if NOT_SORTED is to be used ever for any error >>> other than a tree object sorted incorrectly, or if we start noticing >>> a new error that something is not sorted, we will reuse this one. >> >> s/NOT_SORTED/TREE_&/ maybe? > > If that error is specific to tree sorting order, then that would be > a definite improvement. Yes, that is the case. Tree objects are assumed to list their contents in order, and this ID applies to the problem where a tree object's list is out of order. As I said, I already made both discussed changes part of v6. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in