Skip to content

Replace fmt.Errorf with errors.Errorf#3627

Merged
martinmr merged 4 commits intomasterfrom
martinmr/use-errors-errorf
Jul 11, 2019
Merged

Replace fmt.Errorf with errors.Errorf#3627
martinmr merged 4 commits intomasterfrom
martinmr/use-errors-errorf

Conversation

@martinmr
Copy link
Copy Markdown
Contributor

@martinmr martinmr commented Jul 3, 2019

Fixes #3449


This change is Reviewable

@martinmr martinmr requested review from a team, campoy and danielmai and removed request for a team, gitlw and manishrjain July 3, 2019 18:21
Comment thread dgraph/cmd/migrate/table_guide.go Outdated
campoy
campoy previously requested changes Jul 3, 2019
Copy link
Copy Markdown
Contributor

@campoy campoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 5 of 50 files at r1.
Reviewable status: 5 of 50 files reviewed, 3 unresolved discussions (waiting on @campoy, @danielmai, and @martinmr)


conn/pool.go, line 39 at r2 (raw file):

	ErrNoConnection = errors.Errorf("No connection exists")
	// ErrUnhealthyConnection indicates the connection to a node is unhealthy.
	ErrUnhealthyConnection = errors.Errorf("Unhealthy connection")

These should be errors.New technically ... but I don't think it matters much, tbh


dgraph/cmd/alpha/http.go, line 108 at r2 (raw file):

	uintVal, err := strconv.ParseUint(value, 0, 64)
	if err != nil {
		return 0, errors.Errorf("Error: %+v while parsing %s as uint64", err, name)

These should be replaced with errors.Wrapf(err, "could not parse %q as unit64", name)

And same for all the other fmt.Errorf which embed an error.

Copy link
Copy Markdown
Contributor Author

@martinmr martinmr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 4 of 50 files reviewed, 3 unresolved discussions (waiting on @campoy, @danielmai, and @golangcibot)


conn/pool.go, line 39 at r2 (raw file):

Previously, campoy (Francesc Campoy) wrote…

These should be errors.New technically ... but I don't think it matters much, tbh

Just changed this since they are typed errors. I left the rest as-is.


dgraph/cmd/alpha/http.go, line 108 at r2 (raw file):

Previously, campoy (Francesc Campoy) wrote…

These should be replaced with errors.Wrapf(err, "could not parse %q as unit64", name)

And same for all the other fmt.Errorf which embed an error.

Done.


dgraph/cmd/migrate/table_guide.go, line 24 at r1 (raw file):

Previously, golangcibot (Bot from GolangCI) wrote…

File is not gofmt-ed with -s (from gofmt)


Done.

@martinmr martinmr requested a review from campoy July 3, 2019 23:00
@martinmr martinmr dismissed campoy’s stale review July 9, 2019 18:30

addressed comments

Copy link
Copy Markdown
Contributor

@campoy campoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 44 of 50 files at r1, 1 of 1 files at r2, 22 of 22 files at r3.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @campoy, @danielmai, and @golangcibot)

@martinmr martinmr merged commit 0c68c62 into master Jul 11, 2019
@martinmr martinmr deleted the martinmr/use-errors-errorf branch July 11, 2019 01:08
dna2github pushed a commit to dna2fork/dgraph that referenced this pull request Jul 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Fix calls to fmt.Errorf and errors.Errorf

3 participants