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

Expose DiagnosticGroups.forName(name) as a static method #3309

Conversation

realityforge
Copy link
Contributor

No description provided.

@blickly
Copy link
Contributor

blickly commented Apr 3, 2019

Internal issue created http://b/129796346

@blickly blickly added internal-issue-created An internal Google issue has been created to track this GitHub issue triage-done Has been reviewed by someone on triage rotation. labels Apr 3, 2019
@brad4d brad4d self-assigned this Apr 3, 2019
@brad4d
Copy link
Contributor

brad4d commented Apr 3, 2019

@realityforge I don't have a problem merging this PR, but I presume that this is pre-work for solving some problem. Could you add some explanation here, hopefully with links, to explain why you want this change?

@realityforge
Copy link
Contributor Author

I am away from my development computer atm so this may lack in specifics but essentially google/jsinterop-generator uses closure compiler programatically to read externs. If closure compiler detects an error it should fail but does not atm (See google/jsinterop-generator#28) and this resulted in some errors in downstream projects (i.e. google/elemental2 that went undetected (Mostly as elemental2 patches and selectively imports externs).

To fix this I need to enable new warnings to pick up problems (in this specific case it is both an unknown type error as well as an @override being present without method on parent type and probably more when I get into it)

Currently I have to do something like

    CompilerOptions options = new CompilerOptions();
     options.setWarningLevel(new DiagnosticGroups().forName( "checkTypes" ), CheckLevel.ERROR);

which is just cumbersome. Exposing the method as static seems reasonable as it is a static concept and the 51fd067#diff-657416cab37a79f6ee2356a5bb860d96L105 change does something similar to the getRegisteredGroups()

HTH

@brad4d
Copy link
Contributor

brad4d commented Apr 3, 2019

@realityforge Thanks for the explanation.

PR imported and sent for internal testing and review.

@brad4d
Copy link
Contributor

brad4d commented Apr 4, 2019

Submitted internally.
Should be in tomorrow's push to GitHub.

@brad4d brad4d closed this in 9c3b9c1 Apr 5, 2019
@realityforge realityforge deleted the Expose_DiagnosticGroups.forName_as_static_method branch April 5, 2019 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes internal-issue-created An internal Google issue has been created to track this GitHub issue triage-done Has been reviewed by someone on triage rotation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants