-
Notifications
You must be signed in to change notification settings - Fork 874
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
WIP custom ant task to produce html report from Rat report and NetBeans clusters #70
Conversation
Could you please have a look at the diff of build.xml? It looks as if it got a complete make over. |
And it should be noted, that merging this, will cause merge conflicts for anyone adding exclusions to the rat report. |
build.xml revised. I do a full file xml formating within Netbeans, that was not a good idea. I do a partial formatting and add the exclusion. If merged, the ant task may be updated separatly later. Build.xml should not be impacted any more. |
* @author skygo | ||
*/ | ||
class ModuleInfo { | ||
// approuved resource list |
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 this is a typo: "approved".
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.
thanks
ModuleInfo get = unbymodule.get(mm); | ||
for (String mmu : get.getUnapprouved()) { | ||
bw.write("<li class=\"unapprouved\">"); | ||
bw.write("U:<a href=\"https://github.com/apache/incubator-netbeans/tree/master/" + mmu + "\">" + mmu + "</a>"); |
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.
Hardcoding repository URL is probably not good idea.
Either make the URL an attribute of the task or (preferrably) obtain it from the .git/config
file. There already are places where the build uses hg
command to obtain the commit id (that should be changed to git
one day), so precedent of using the repo information from builds exists.
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.
nice addition thanks. will look at it
nbbuild/build.xml
Outdated
</fileset> | ||
</rat:report> | ||
<taskdef name="rattohtml" classname="org.netbeans.nbbuild.reporting.RatHtmlReportTask" classpath="${build.ant.classes.dir}"/> | ||
<loadproperties srcFile="cluster.properties" /> | ||
<rattohtml source="build/rat-report.xml" target="build/rat-report.html"/> |
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 am not sure whether HTML is that much better format than txt. Probably the direct hyperlinks to the Git repo are the biggest improvement, right?
In any case I was looking for a way to track progress of our rat related efforts. Instead of HTML the task should generate JUnit XML report files. If the output used JUnitReportWritter just like the other classes in the nbbuild/antsrc
, we could set the continuous job to show us how many violations we still have and track the progress.
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.
Sorry but the last commit with refactoring kill the converstation flow.
Maybe hyperlink is for now the only improvement for having HTML report. By cluster information can be done in txt file.
Helping tooling continuous job is a big point. But we should also think on people who will review and vote for release.
for example http://commons.apache.org/proper/commons-bcel/ has a project information section including a rat report.
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
package org.netbeans.nbbuild.reporting; |
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.
Does this really deserve a separate package? There is a lot of reporting tasks already present in org.netbeans.nbbuild
... Moving into org.netbeans.nbbuild
and merging ModuleInfo
to be static inner class would, in my opinion, be more consistent.
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.
This sounds good for me
bw.write("<h3>Module::" + mm + "</h3>"); | ||
bw.write("<ul>"); | ||
ModuleInfo get = unbymodule.get(mm); | ||
for (String mmu : get.getUnapprouved()) { |
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.
approuved
typos are everywhere...
I had to check on personal jenkins to see what I get from junit report. I hope the mutliple commit and rebase were not notified. The report are by clusters in a rat folder in case we want to filter them later.
|
@JaroslavTulach I think the WIP is in now in a stable state. rebasing breaks in place review sorry for that :/ |
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.
Once integrated we have to use this in our Jenkins jobs. I can administered them...
} | ||
|
||
@Override | ||
public void execute() throws BuildException { |
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.
Super.execute is probably useless.
commandAndArgs.add("config"); | ||
commandAndArgs.add("--get"); | ||
commandAndArgs.add("remote.origin.url"); | ||
Process p = new ProcessBuilder(commandAndArgs).directory(root).start(); |
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.
R is never closed
// XXx if amoduleInfo is null folder is not present source base incomplete | ||
} | ||
|
||
} |
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.
Nice.
@jtulach rebased again but rat task have more than only fileset from a PR by @matthiasblaesing |
get git information from git process add scan for external
@jtulach or @JaroslavTulach , rebasing of build.xml is annoying. |
@JaroslavTulach |
OK, I try to merge. |
Integrated as 162850d - builds modified to pick the generated .xml files up. |
The build is failing: https://builds.apache.org/job/incubator-netbeans-linux/114/console |
Strange, |
parsing xml with file seems to be dangerous for encoding. #144 use InputSource. Seems to fix the issue on my new non UTF-8 image. |
I think the problem is in rat - with an output file this is what happens:
if (reportFile == null) {
out = new PrintWriter(
new OutputStreamWriter(
new LogOutputStream(this, Project.MSG_INFO)
)
);
} else {
out = new PrintWriter(new FileWriter(reportFile));
}
createReport(o The prolog written in step 3 would be wrong in many cases as it does not write an encoding info. This means the encoding of the resulting document must be UTF-8. The FileWriter created in step 1 uses the default charset and if that is not UTF-8 a broken stream will result. The Report Task would need to be changed from (line 196): out = new PrintWriter(new FileWriter(reportFile)); to out = new PrintWriter(new OutputStreamWriter(new FileOutputStream(reportFile), Charset.forName("UTF-8"))); |
Hi,
this is a more deep integration with Netbeans of the rat report.
It filter the report using cluster.properties and check for local folder.
The filtering pattern is now a variable to get same result for txt and html report to keep the current txt report intact.
Regards