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

DM-35821: Fix CTI run errors #224

Merged
merged 1 commit into from Aug 3, 2022
Merged

DM-35821: Fix CTI run errors #224

merged 1 commit into from Aug 3, 2022

Conversation

czwa
Copy link
Contributor

@czwa czwa commented Aug 3, 2022

Fix errors in creating CTI statistics.

Copy link
Contributor

@plazas plazas left a comment

Choose a reason for hiding this comment

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

This looks good to me as long as it fixes the errors reported in Jira. I think I understand better the fix to serialize the data (it needs to be a list, it seems), but I'm not sure why this check is not needed anymore. Also, shouldn't these errors have been captured by some sort of unit testing?

@czwa
Copy link
Contributor Author

czwa commented Aug 3, 2022

This looks good to me as long as it fixes the errors reported in Jira. I think I understand better the fix to serialize the data (it needs to be a list, it seems), but I'm not sure why this check is not needed anymore. Also, shouldn't these errors have been captured by some sort of unit testing?

The additional check was added during review, and I wasn't sure at the time that it was needed. Unfortunately, the testing on the previous ticket was focused on the Solve stage, and so these bugs slipped through.

@czwa czwa merged commit 673fa65 into main Aug 3, 2022
@czwa czwa deleted the tickets/DM-35821 branch August 3, 2022 22:59
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

2 participants