BLADE-516 Add 5 sec timeout to automatic update check #45
Conversation
BLADE-507 Use latest project-templates
BLADE-509 fix tests
BLADE-513 switch to 7.3 default
- Use Ant to load build.properties - For any jar that we don't know GAV try to look it up in META-INF
BLADE-510 improve GAV dependency generation
|
@christopherbryanboyd, can I get your review on this? |
| } | ||
| catch (Exception e) { | ||
| e.printStackTrace(bladeCLI.error()); | ||
| } |
There was a problem hiding this comment.
Perhaps we should throw these as RuntimeExceptions and let the top level try/catch handle them?
Essentially they do the same thing, printing the error, so that would unify the behavior.
| catch (Exception e) { | ||
| e.printStackTrace(bladeCLI.error()); | ||
| } | ||
| }); | ||
|
|
||
| Stream<File> webPluginStream = webPlugins.stream(); | ||
|
|
There was a problem hiding this comment.
Perhaps these streams should be wrapped in a try-with-resources block?
Otherwise if an exception is thrown, the stream might not get closed.
| } | ||
| } | ||
| catch (IOException e) { | ||
| } |
There was a problem hiding this comment.
Do we want to do anything with this exception?
| try { | ||
| warsDir.mkdirs(); | ||
| private void _convertToWarProject(File pluginsSdkDir, File warsDir, File pluginDir, boolean removeSource) | ||
| throws Exception { | ||
|
|
There was a problem hiding this comment.
This is a rather large method, probably not a big deal right now but eventually I think we should consider refactoring this.
| } | ||
|
|
||
| private static class GAV { | ||
|
|
There was a problem hiding this comment.
I don't think it is good Java naming conventions to have a fully upper-case class, perhaps we could rename it GroupArtifactVersion, or something like that?
| _artifactId = Optional.ofNullable(artifactId); | ||
| _version = Optional.ofNullable(version); | ||
| } | ||
|
|
There was a problem hiding this comment.
Why are these all Object rather than String?
| @@ -144,7 +144,7 @@ public void testLoadCommandsWithCustomExtensionInWorkspace() throws Exception { | |||
|
|
|||
| File workspaceDir = temporaryFolder.newFolder("build", "test", "workspace"); | |||
|
|
|||
| String[] args = {"--base", workspaceDir.getPath(), "init", "-P", "foo", "-v", "7.2"}; | |||
| String[] args = {"--base", workspaceDir.getPath(), "init", "-P", "foo", "-v", "7.3"}; | |||
|
|
|||
There was a problem hiding this comment.
Should we have both 7.2 and 7.3 tests, or is it good enough to just test 7.3?
No description provided.