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

Do not add errors key when empty #766

Merged
merged 4 commits into from
Jan 23, 2021
Merged

Do not add errors key when empty #766

merged 4 commits into from
Jan 23, 2021

Conversation

TamasSzigeti
Copy link
Contributor

If the error handler filters out all the errors so that none remains, we should not add the errors key to the result.

@coveralls
Copy link

coveralls commented Jan 13, 2021

Coverage Status

Coverage increased (+0.003%) to 86.189% when pulling 4d0641c on lingoda:no-empty-errors into 5289a54 on webonyx:master.

Copy link
Collaborator

@spawnia spawnia left a comment

Choose a reason for hiding this comment

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

I just added some minutiae for clarity, the change makes sense to me.

@spawnia spawnia added the bug label Jan 13, 2021
@TamasSzigeti
Copy link
Contributor Author

I just added some minutiae for clarity, the change makes sense to me.

Thank you, done both.

TamasSzigeti and others added 2 commits January 14, 2021 08:09
Co-authored-by: Simon Podlipsky <simon@podlipsky.net>
Copy link
Collaborator

@simPod simPod left a comment

Choose a reason for hiding this comment

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

Just checked and complies with spec 👍🏾

Copy link
Collaborator

@spawnia spawnia left a comment

Choose a reason for hiding this comment

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

Nice

@TamasSzigeti
Copy link
Contributor Author

Can we merge and tag this?

@spawnia spawnia requested a review from vladar January 22, 2021 14:18
@vladar vladar merged commit 3b74335 into webonyx:master Jan 23, 2021
Comment on lines +150 to +153
// While we know that there were errors initially, they might have been discarded
if ($handledErrors !== []) {
$result['errors'] = $handledErrors;
}

This comment was marked as resolved.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Handling the errors could also mean to translate the messages, format them nicely, whatever!

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see :D Yeah then I suppose it makes sense 🙂

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.

6 participants