Hi Martin, comments inline.
On Aug 15, 2011, at 3:50 AM, Martin Grigorov wrote:
> Is it possible to configure the plugin to fail on unknown files ?
There are no limitations of what we can do, even if we need to fork the mycila
plugin to do it. :-) But if you mean "can the plugin as written deal with
unknown files, the only way to do that would be to include everything
(<include>**</include>) and then write an exclude that matches every file that
should not be included. This will catch new files that are unknown in the way
you mentioned. I didn't come to believe that the old system did this though,
just that it handled the six or so types (java, js, xml, vm, css, and a couple
others I don't remember ATM).
> E.g. currently it excludes 'target/' folder and checks files with
> extensions .java, .xml, .html, .properties, .js, .css and .vm. Is it
> possible to add some global excludes for images and configure it to
> fail the verification if an unknown file is found. I.e. if there is
> src/main/java/org/apache/wicket/SomeFile.blah and it is not explicitly
> excluded then fail
My experience with the old method of using unit tests to test that the source
code had headers was not an exact process either. I had seen many files that
did not have headers for no apparent reason. What I tried to do was find a
"best fit" to the existing rules. With as many files as there are in the
system, it would require an automated test rig to ensure that every file was
being managed exactly like the old system did, and given that the old system
had holes caught on visual inspection, it didn't seem that was time well-spent.
>
> Additionally can you add some comments around why some resources are excluded
> ?
> I just saw wicket-core/pom.xml has:
> <plugin>
> 83
> + <groupId>com.mycila.maven-license-plugin</groupId>
> 84
> + <artifactId>maven-license-plugin</artifactId>
> 85
> + <configuration>
> 86
> +
> <header>${project.parent.basedir}/licenses/asf-license-header.txt</header>
> 87
> + <excludes>
> 88
> +
> <exclude>src/main/java/org/apache/wicket/**/*.properties</exclude>
>
> and I wonder why line 88 is there.
Yes, line 88 is probably incorrect. I'll take a look at it.
> I know current JUnit based tests also have some exclusions but I'm not
> sure whether they have explanation "why".
The issue is with test data. For instance, a .html file in the test tree may
not actually be used in the compilation of a test, it may be just for
comparison that WicketTester created the final output that is correct. Adding
a header to a file like that would cause the comparison (and the test) to fail.
We could add that header to both the test input and test output, but I was
trying to take the path of least impact.
>
> On Mon, Aug 15, 2011 at 10:06 AM, Martin Grigorov <[email protected]>
> wrote:
>> Hi,
>>
>> The change at
>> https://github.com/topping/wicket/commit/8f08c2082f90fcd54a7e4734b892d13e610a47bf
>> looks OK to me but I'm not quite familiar with the unit test based
>> solution (the current one). I'm not sure whether there are some
>> (unknown for me) rules like specific formatting which may become
>> broken now.
This doesn't seem to be the case. I'm committed to supporting the change that
I made here though, so whatever minor changes are necessary, I'm happy to have
issues assigned to me.
>>
>> I guess Andreas' solution will be needed at least for wicket-core
>> because as I said (Base)WicketTester depends on JUnit - both compile
>> and runtime.
Yes, the best practice in this case is to have a test module containing test
code that has a runtime dependency on a library such as JUnit. There was a
huge amount of changes to get this in, and I overlooked that o.a.w.util.tester
is in core. Having it remain that way would cause the entire body of work to
be pretty, but not very useful to it's original end.
Are there practical reasons that o.a.w.util.tester is in core?
<snip/>
Cheers, Brian
