-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Implement assert() messages behind a flag in analyzer #24217
Comments
https://codereview.chromium.org/1309543011/ adds support to the analysis engine. Once that has been reviewed and landed, we will need to make a separate (trivial) changes to analyzer_cli and analysis_server to add the flag. |
Thanks! Should I open a new issue to track the other plumbing? Update: I just did :) |
And you added tests. 🤘 |
The DEP met today, and decided that the type annotation on the optional param should be String. Does the CL reflect that? |
IIRC, the CL does reflect that. However I can't verify right now since codereview.chromium.org appears to be down due to certificate errors. I will check back tomorrow to confirm. |
Thanks Paul. |
I just double-checked, and yes, the CL is consistent with the proposed spec change. It requires the optional assertion message param to be assignable to String, or a warning is issued. |
Thanks @stereotype441 for checking! |
Messages in asserts is now enabled by default in analyzer (https://codereview.chromium.org/2557513008/). |
This is the analyzer-specific issue for #24213. That issue has the details.
The text was updated successfully, but these errors were encountered: