Skip to content

Improve the content type error message by showing available content types#3532

Merged
gitlw merged 2 commits intomasterfrom
gitlw/improve_content_type_err
Jun 7, 2019
Merged

Improve the content type error message by showing available content types#3532
gitlw merged 2 commits intomasterfrom
gitlw/improve_content_type_err

Conversation

@gitlw
Copy link
Copy Markdown

@gitlw gitlw commented Jun 6, 2019

This change is Reviewable

@gitlw gitlw requested a review from manishrjain as a code owner June 6, 2019 22:24
@gitlw gitlw requested a review from a team June 6, 2019 22:24
Copy link
Copy Markdown
Contributor

@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.

:lgtm:

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gitlw and @manishrjain)


dgraph/cmd/alpha/http.go, line 204 at r1 (raw file):

	default:
		x.SetStatus(w, x.ErrorInvalidRequest, "Unsupported content type. "+
			"Currently supported content types are application/json, application/graphqlpm")

minor: I think it's safe to remove the currently to make the message a bit shorter. It's implied that the message is referring to the current types.

Copy link
Copy Markdown
Contributor

@danielmai danielmai 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: all files reviewed, 3 unresolved discussions (waiting on @gitlw and @manishrjain)


dgraph/cmd/alpha/http.go, line 203 at r1 (raw file):

	default:
		x.SetStatus(w, x.ErrorInvalidRequest, "Unsupported content type. "+

Let's keep the original spelling of "Content-Type" so it's clearer this is referring to the header name.


dgraph/cmd/alpha/http.go, line 331 at r1 (raw file):

	default:
		x.SetStatus(w, x.ErrorInvalidRequest, "Unsupported content type. "+

Let's keep the original spelling of "Content-Type" so it's clearer this is referring to the header name.

Copy link
Copy Markdown
Author

@gitlw gitlw 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: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @danielmai, @manishrjain, and @martinmr)


dgraph/cmd/alpha/http.go, line 203 at r1 (raw file):

Previously, danielmai (Daniel Mai) wrote…

Let's keep the original spelling of "Content-Type" so it's clearer this is referring to the header name.

Done.


dgraph/cmd/alpha/http.go, line 331 at r1 (raw file):

Previously, danielmai (Daniel Mai) wrote…

Let's keep the original spelling of "Content-Type" so it's clearer this is referring to the header name.

Done.

@gitlw gitlw merged commit 9cac881 into master Jun 7, 2019
@gitlw gitlw deleted the gitlw/improve_content_type_err branch June 7, 2019 00:31
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.

3 participants