Skip to content

Move json_error to error.message and error.type#4167

Merged
tsg merged 1 commit intoelastic:masterfrom
ruflin:error.json
May 10, 2017
Merged

Move json_error to error.message and error.type#4167
tsg merged 1 commit intoelastic:masterfrom
ruflin:error.json

Conversation

@ruflin
Copy link
Copy Markdown
Contributor

@ruflin ruflin commented May 2, 2017

All errors are now under the error namespace. This should also be true for json errors. Currently they are put under json_error. This PR changes it to error.message and error.type: json. This makes it easy to search for all json errors. This is a breaking change.

  • Docs updated
  • Tests updated

The naming of the config option was not touched for better backward compatibility.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this have to be exported? Seems simple enough to replicate if we need it elsewhere, similar to the debug function calls.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I exported it because I also use it in the json reader. But I can also just copy / paste the code to the json reader.

Copy link
Copy Markdown
Contributor

@tsg tsg left a comment

Choose a reason for hiding this comment

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

LGTM. See tiny comment and please add PR number to changelog.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[golint] reported by reviewdog 🐶
exported function WriteJSONKeys should have comment or be unexported

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[golint] reported by reviewdog 🐶
exported function CreateJSONError should have comment or be unexported

@ruflin
Copy link
Copy Markdown
Contributor Author

ruflin commented May 2, 2017

PR added, function duplicated and golint errors fixed.

@ruflin
Copy link
Copy Markdown
Contributor Author

ruflin commented May 2, 2017

jenkins, retest it

@ruflin ruflin force-pushed the error.json branch 2 times, most recently from e480942 to 840e1e7 Compare May 4, 2017 07:10
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[golint] reported by reviewdog 🐶
comment on exported function WriteJSONKeys should be of the form "WriteJSONKeys ..."

@ruflin ruflin force-pushed the error.json branch 4 times, most recently from a578ca5 to 183e793 Compare May 9, 2017 13:13
All errors are now under the error namespace. This should also be true for json errors. Currently they are put under `json_error`. This PR changes it to `error.message` and `error.type: json`. This makes it easy to search for all json errors. This is a breaking change.

* Docs updated
* Tests updated

The naming of the config option was not touched for better backward compatibility.
@tsg tsg merged commit 97a549a into elastic:master May 10, 2017
@tsg tsg mentioned this pull request Jul 24, 2017
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants