Johannes Schindelin [off-list ref] writes:
I basically made up names on the go, based on the messages.
quoted
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.
quoted
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.
quoted
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.
Thanks.
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 [off-list ref] writes:
quoted
I basically made up names on the go, based on the messages.
quoted
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.
quoted
quoted
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.
quoted
quoted
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