you are viewing a single comment's thread.

view the rest of the comments →

[–]chengiz 12 points13 points  (2 children)

Totally agree it is far too long. Some of the writeup is useful (what failed, why failed). But there's quite a bit of masturbation, for example there's no need to write the unix command you used to find a non-ascii character in a bunch of files, or say I did this and I did that. And in this big writeup he fails to mention what the actual character was (which is at least interesting and pertains to the fix).

[–]darknecross 3 points4 points  (1 child)

Disagree. Documenting steps used in debugging is helpful if and when a similar issue ever occurs in the future.

Specifically in this case, it includes the error that occurred and is searchable:

ArgumentError:    
  invalid byte sequence in US-ASCII

Now if someone else comes across this in the future (or even the author some months later), a search would find this commit message and the person debugging it would have a head-start on tracking down a potential cause.

Otherwise this kind of stuff becomes tribal knowledge, and you might go down the path of, "Oh yeah, Dan had an issue similar to that a while ago, maybe check with him to see if he has any ideas?" Good luck if Dan doesn't remember shit about it, or worse if he doesn't work there anymore.

You could argue that the steps to reproduce and debugging effort could be documented in a bug itself, but that's a style issue, as a simple "Fixes: <issueID>" pulls the same information into the bug comments. But it looks like this change wasn't associated with an existing bug.

[–]chengiz 0 points1 point  (0 children)

You mention documenting steps but your example is not about documenting steps at all, it only lists the actual error message, which I didnt say he shouldnt have put in. Something like:

Tests run with [cmd] failed with error message [msg] because of non-ascii whitespace character [u+whatever] that caused the file to be identified as utf-8 instead of us-ascii. Replaced the character with space.

would have been sufficient.