Added Email to header #9
Conversation
|
Any reason for making |
There was a problem hiding this comment.
Few comments, PTAL.
Note build fails (findbugs):
$ mvn clean verify
<snip>
...
[INFO] --- findbugs-maven-plugin:3.0.4:check (findbugs) @ bayesian ---
[INFO] BugInstance size is 4
[INFO] Error size is 0
[INFO] Total bugs: 4
[INFO] Found reliance on default encoding in com.redhat.jenkins.plugins.bayesian.Bayesian.getEmail(): new java.io.InputStreamReader(InputStream) [com.redhat.jenkins.plugins.bayesian.Bayesian] At Bayesian.java:[line 171] DM_DEFAULT_ENCODING
[INFO] com.redhat.jenkins.plugins.bayesian.Bayesian.getEmail() may fail to close stream [com.redhat.jenkins.plugins.bayesian.Bayesian] At Bayesian.java:[line 171] OS_OPEN_STREAM
[INFO] Should com.redhat.jenkins.plugins.bayesian.User$Attributes be a _static_ inner class? [com.redhat.jenkins.plugins.bayesian.User$Attributes] At User.java:[lines 19-28] SIC_INNER_SHOULD_BE_STATIC
[INFO] Should com.redhat.jenkins.plugins.bayesian.User$Data be a _static_ inner class? [com.redhat.jenkins.plugins.bayesian.User$Data] At User.java:[lines 32-45] SIC_INNER_SHOULD_BE_STATIC
[INFO]
To see bug detail using the Findbugs GUI, use the following command "mvn findbugs:gui"
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 28.494 s
[INFO] Finished at: 2018-05-02T23:39:16+02:00
[INFO] Final Memory: 88M/1429M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.codehaus.mojo:findbugs-maven-plugin:3.0.4:check (findbugs) on project bayesian: failed with 4 bugs and 0 errors -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
pom.xml
Outdated
| @@ -78,6 +78,11 @@ | |||
| <artifactId>dnsjava</artifactId> | |||
| <version>2.1.8</version> | |||
| </dependency> | |||
| <dependency> | |||
| <groupId>commons-logging</groupId> | |||
There was a problem hiding this comment.
I don't see this dependency used in the code - do we need it?
There was a problem hiding this comment.
HttpGet was using this internally and build was failing without this dependency. Hence i added it
| @@ -46,6 +50,8 @@ | |||
| /* package */ class Bayesian { | |||
|
|
|||
| private static final String DEFAULT_BAYESIAN_URL = "https://recommender.api.openshift.io/"; | |||
| private static final String OSIO_USERS_URL = "https://api.openshift.io/api/users"; | |||
There was a problem hiding this comment.
This is good enough for testing purposes, but the URL should be configurable as the value is specific to prod deployment.
| CloseableHttpResponse response = client.execute(httpGet)) { | ||
|
|
||
| HttpEntity entity = response.getEntity(); | ||
| InputStream is = entity.getContent(); |
There was a problem hiding this comment.
I believe you're mixing tabs and spaces in this method. Please stick with spaces :)
There was a problem hiding this comment.
I think this InputStream should be closed once we are done with it.
| try (CloseableHttpClient client = HttpClients.createDefault(); | ||
| CloseableHttpResponse response = client.execute(httpGet)) { | ||
|
|
||
| HttpEntity entity = response.getEntity(); |
|
|
||
| gson = new GsonBuilder().create(); | ||
|
|
||
| responseObj = gson.fromJson(sb.toString(), User.class); |
There was a problem hiding this comment.
We do something similar in this file already: https://github.com/yzainee/fabric8-analytics-jenkins-plugin/blob/d5af23f1f2baa49cebdddbd5c1653388fe53e8bc/src/main/java/com/redhat/jenkins/plugins/bayesian/Bayesian.java#L119-L125
I believe you could reuse the same approach here. try-with-resources would also take care of closing the input stream for you.
No description provided.