-
Notifications
You must be signed in to change notification settings - Fork 22
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
coverage#CreateReport: move assertions up #23
Conversation
This makes it easier in case of errors to see where it is coming from.
Can you give more context, like what the error looks like w/ and w/o? We use this convention widely across all our plugins, and it'd be unfortunate if the errors aren't precise, but also I'm hesitant to start unwinding this kind of thing everywhere it appears and I don't see any special reason we'd do it here and not everywhere else. |
Currently I am seeing:
(using google/vim-maktaba#213 to fix the type name) The error will be the same, but it's easier to figure out where it's coming, since you can easily comment the lines. Not very helpful altogether, but I guess that's where this PR comes from. When throwing a custom exception you see that it is coming via Therefore additionally/instead |
The error/issue I was seeing: #30 |
Sounds like you're saying the issue is that there are 3 |
Yup, thanks! |
(I approved. You have a button to merge the changes now, right?) |
No. |
I approved them so I thought you could. Anyway, I merged them for you. |
This makes it easier in case of errors to see where it is coming from.