-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rename error variable in go tests to err #8828 #11716
base: main
Are you sure you want to change the base?
Conversation
renamed the variables from e to err as mentioned in the https://go.dev/doc/effective_go
Result of foundationdb-pr-clang-ide on Linux CentOS 7
|
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr on Linux CentOS 7
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit. LGTM
Result of foundationdb-pr-clang-ide on Linux CentOS 7
|
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr on Linux CentOS 7
|
bindings/go/src/fdb/doc.go
Outdated
fmt.Printf("hello is now world, foo was: %s\n", string(ret.([]byte))) | ||
} | ||
fmt.Printf("hello is now world, foo was: %s\n", string(ret.([]byte))) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these whitespace changes intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be because of the default formatter. They were not intentional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i have removed these whitespaces.
@@ -115,7 +115,7 @@ func (de *DirectoryExtension) processOp(sm *StackMachine, op string, isDB bool, | |||
} | |||
}() | |||
|
|||
var e error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be removed altogether?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes!
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-clang-ide on Linux CentOS 7
|
Looks like there are CI errors:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR. The next time it would be great to have two PR's for such things, e.g. one PR where you introduce the formatting and another PR where the variable is renamed, otherwise you'll have a huge PR which is hard to review.
@@ -90,7 +90,7 @@ func (d directorySubspace) Exists(rt fdb.ReadTransactor, path []string) (bool, e | |||
return d.dl.Exists(rt, d.dl.partitionSubpath(d.path, path)) | |||
} | |||
|
|||
func (d directorySubspace) List(rt fdb.ReadTransactor, path []string) (subdirs []string, e error) { | |||
func (d directorySubspace) List(rt fdb.ReadTransactor, path []string) (subdirs []string, err error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func (d directorySubspace) List(rt fdb.ReadTransactor, path []string) (subdirs []string, err error) { | |
func (d directorySubspace) List(rt fdb.ReadTransactor, path []string) (]string, error) { |
I don't see a reason why we need a named return value here.
Result of foundationdb-pr-clang-ide on Linux CentOS 7
|
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr on Linux CentOS 7
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 Thanks for the PR!
Renamed the variables from e to err as mentioned in the https://go.dev/doc/effective_go
According to the issue number #8828
the variables in go code were named using good and effective go practices.
Following the example link for reference mentioned in the issue the variables have been renamed.