remove panic calls #227
Merged
remove panic calls #227
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
dmatch01 requested changes Oct 18, 2022
Collaborator
dmatch01 left a comment
dmatch01 left a comment There was a problem hiding this comment.
We need a way to debug this problem in the since this is a workaround. 1 of the 2 options below is suggest. I would recommended option 1 because there it is efficient in logging only when there is a problem.
- Change functions that had
panicto also return anerroras a second parameter and have the caller provide logging information about the issue. - Every entity that calls these previously
panicfunctions will need to log info. before calling. So for instance that line number you found in the bug. Those lines of code will need to generate log entries to generate details to debug the problem.
Signed-off-by: dmatch01 <darroyo@us.ibm.com>
Member Author
| Thanks @dmatch01 , LGTM! |
dmatch01 approved these changes Nov 7, 2022
Collaborator
dmatch01 left a comment
dmatch01 left a comment There was a problem hiding this comment.
Thanks for doing the test case. I have a Q about this test case. I'm not quite how it confirms there is not panic. In otherwards: If I take this test case and add it to the previous version of code will it pass/fail? If you can check this that would be great. Otherwise if it still passes on the older code can you add a issue in mcad to fix this test case? That way I can go ahead and approve with merge. Thank you!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
remove panic calls