-
Notifications
You must be signed in to change notification settings - Fork 16
Conversation
@@ -0,0 +1,27 @@ | |||
package com.org.jenkins.Custom.Jenkins.Distribution.Service; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue found: Package name contains upper case characters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can just ignore this for now. Because the changes have already been made in the other PR #28
The update center is too big to download and as @LinuxSuRen rightly pointed it will increase the size of the repo and more importantly it is updated every time, should we just call the function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think we should. There's a ticket for working with caching for later in the coding period. For now, let's just download every time.
Let's get the other PR merged before we come back to this. That will also give you the chance to move to using this file instead of the static update center. I think also when this class is ready to work into the main checker, you can write some tests to make sure this all works. You can use the modified/reduced update center in your tests.
|
||
private static final String UPDATE_CENTER_JSON_URL = "https://updates.jenkins.io/current/update-center.actual.json"; | ||
|
||
public void downloadUpdateCenterJSON() throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventually you might want to pull this out to a config file to make it customisable for someone who wants to run the tool against a custom update center. I'm fine with this being here now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I agree, so that the user can put in the customized url in a config file and then the file downloader can just use that url
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
This PR downloads the
actual-update-center.json
file and stores it into resources.