Skip to content
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

Now errors with context have the same schema. #519

Merged
merged 1 commit into from Jun 21, 2016

Conversation

dgaubert
Copy link
Contributor

please guys, could you CR this?

cc @rochoa & @alonsogarciapablo

@alonsogarciapablo
Copy link
Contributor

👍

@dgaubert dgaubert merged commit 256032c into master Jun 21, 2016
dgaubert pushed a commit that referenced this pull request Jun 21, 2016
var error = new Error(err.message);
error.type = 'analysis';
error.context = {};
error.context.layer = {
Copy link
Contributor

Choose a reason for hiding this comment

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

@dgaubert why error.context.layer and not just error.context?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's kind of weird cause the id is actually the id of the analysis, but it's inside of the layer property...

Copy link
Contributor Author

@dgaubert dgaubert Jun 26, 2016

Choose a reason for hiding this comment

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

Right, it does not make sense. I think is better if you provide an example of what do you need for both analysis and layer (turbo-carto) errors. Because, at the end, you know better what's needed for this issue.

@rochoa rochoa deleted the fix-error-with-context branch December 5, 2016 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants