Applied APIView suggestion for KV Admin. #22381
Conversation
ghost
added
the
| try { | ||
| return fromString(new URL(url).getPath(), KeyVaultRoleScope.class); | ||
| } catch (MalformedURLException e) { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
Should still document that an exception is thrown by the method even if it isn't checked. Also, should this be an IllegalArgumentException to be more targeted?
There was a problem hiding this comment.
I think IllegalArgumentException is fine but since it a static method I'm not sure if( we should use a logger or not. I don't think we want to have static loggers. Any thoughts @srngar?
There was a problem hiding this comment.
Not using a logger for now, but did change to the exception type you suggested
There was a problem hiding this comment.
IllegalArgumentException should be logged and thrown here. You can use static final loggers in static methods.
|
/azp run java - keyvault |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/check-enforcer evaluate |
|
/check-enforcer reset |
|
/check-enforcer evaluate |
|
/check-enforcer override |
And other small changes.