[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #31794 [Circumvention/Snowflake]: Errors swallowed
#31794: Errors swallowed
-------------------------------------+------------------------------
Reporter: sah | Owner: (none)
Type: defect | Status: needs_review
Priority: Medium | Milestone:
Component: Circumvention/Snowflake | Version:
Severity: Normal | Resolution:
Keywords: | Actual Points:
Parent ID: | Points:
Reviewer: cohosh | Sponsor:
-------------------------------------+------------------------------
Comment (by sah):
Hi dcf, thanks for taking the time to review the ticket.
WRT Error strings.
I refer you to the widely cited
https://github.com/golang/go/wiki/CodeReviewComments#error-strings
Error strings should not be capitalized (unless beginning with proper
nouns or acronyms) or end with punctuation, since they are usually printed
following other context. That is, use fmt.Errorf("something bad") not
fmt.Errorf("Something bad"), so that log.Printf("Reading %s: %v",
filename, err) formats without a spurious capital letter mid-message. This
does not apply to logging, which is implicitly line-oriented and not
combined inside other messages.
I opted to exempt these instances from the rule for two reasons.
Firstly: They are succinct, and it's absolutely clear to the user
receiving the error what's wrong, and where the problem is.
Secondly: It's also clear that they are temporary, that is, the error
message will be removed once the functions/methods are implemented.
Where I have changed them, I don't think I should have and will go back
and reset them.
With respect to the explicit nil value. It's unnecessary to have that code
exist, one of the features of Go is that no variable is uninitialised. I
don't think that the explicitness provides any advantage for the
maintainer, because it assumes that they do not know how to code in Go.
WRT shadowed err:
The confusion caused by the shadowing is evident in your comment, was it
deliberate or not? When there is a desire for the inner scoped error to be
separate and distinct from the outer scoped error then a separate and
distinct identifier is appropriate. (Curiously this is the reverse of the
explicit setting of the nil value for the variables mentioned in the
previous paragraph)
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/31794#comment:14>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs