-
Notifications
You must be signed in to change notification settings - Fork 26
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
JBDS-3929 apply the same content.xml... #56
base: jbosstools-4.4.0.x
Are you sure you want to change the base?
Conversation
Didn't try it and cannot do it right now, but it seems fine in principle. |
PR revised. Need to do the same transform for artifacts.* too. Also refactored some variables. |
…s in content.xml.xz JBDS-3929 apply the same artifacts.xml transformations in artifacts.jar as in artifacts.xml.xz
// see also https://bugs.eclipse.org/bugs/show_bug.cgi?id=464614 | ||
//getLog().debug("delete old artifacts.xml.xz"); | ||
File artifactsXmlXz = new File(p2repository,"artifacts.xml.xz"); | ||
FileUtils.forceDelete(artifactsXmlXz); |
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.
Should this code for .xz'en the file be put in a separate method instead of duplicating that many lines ?
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.
Done. Thx.
I can't test it but the intent is fine but I would from the start avoid the big duplication of the code to do the .xz compression. +1 when thats cleaned up. |
while (-1 != (n = in.read(buffer))) { | ||
out.write(buffer, 0, n); | ||
} | ||
out.close(); |
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.
no try/catch or similar to report to user what file possibly failed
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.
extracted method alterXzFile throws IOException, FileNotFoundException, TransformerException ... is that enough or do you still need try/catch blocks to trap for these errors and dump more details to console?
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.
neither of those exceptions will actually tell you what file the issue is with. I would recommend catching them and rethrowing with them nested inside a Tycho or Maven based runtime exception that will have a message that will help you actually track down what is the problem.
BTW to test this, just mvn clean install it locally, then go into jbdevstudio-product/ from master branch and mvn clean install there too. Results are in site/target/repository, which you can then browse from Eclipse or JBDS to see what's available in the update site. |
JBDS-3929 apply the same content.xml transformations in content.jar as in content.xml.xz