-
-
Notifications
You must be signed in to change notification settings - Fork 358
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
fix: comment of CtAnnotation value #2587
Conversation
It is not a fix, you basically remove the log. I strongly disagree with this solution. This log is designed to detect thoses cases. |
It's not only removing the log, the comment is attached to an element, so there is a new best-effort behavior. Are you against this best-effort behavior? |
I'm against this one because from my experience the computed parent of comment can be pretty far from an acceptable target. And also because you removed the only way to detect those buggy case. From my point of view, this log is one of the best logs you can have. When this log is printed it means that there is a bug in the application. The solution is to fix the bug that the log you want to remove detected. |
I see what you mean. I've deleted the best effort idea and restored the log. I've isolated the problem in a failing test case, could you have a look at it? Thanks! |
@pvojtechovsky could you have a look at this one? |
ok, I will have a look |
1cac983
to
82a7692
Compare
rebase was needed. I did it |
Many thanks Pavel! |
You are welcome Martin ;-) |
fix #2209
Note that it's a partial fix, the comment should be added to the expression inside the annotation, but it's still much better than the dirty exception in logs of before.