I'm checking it right now.
Sven
On 11/04/2013 01:01 PM, Martin Grigorov wrote:
Martijn started a new mail thread with subject "Autoformatting and eclipse
settings".
There he changed the topic to something like "the problem is in non-Eclipse
developers who break the formatting of the code and thus Eclipse users
should fix it after them". I suggest to keep that topic in that other
thread.
In this one I'd like to ask another Wicket developer who uses Eclipse to
review PR 56 and either merge it or reject it. That's all.
Otherwise we may kill the enthusiasm of contributors like Martin Funk.
On Mon, Nov 4, 2013 at 10:58 AM, Martin Grigorov <[email protected]>wrote:
Hi,
Can someone of other Wicket code developers take a look at
https://github.com/apache/wicket/pull/56 ?
This is a pull request with some changes/updates to Eclipse's .settings/
(required by newer versions of Eclipse ?!).
I don't use Eclipse and I cannot decide whether the PR is good or not.
https://github.com/apache/wicket/pull/57/commits is another PR from
Martin Funk that has some improvements to Wicket's unit tests that I'd like
to merge but I cannot because it depends on PR 56.
Additionally I'd like to ask all Eclipse users to disable the "auto format
the whole file" feature.
https://github.com/mafulafunk/wicket/commit/0aac81f393047865088864c6b299ce1e022ce1fa
(part
of PR 57) has such formatting changes that we agreed should not be together
with functional changes because they add a lot of noise that makes the code
review and git bisect sesssions a lot harder.
Lately I have seen such changes in Sven's commits as well.
Please configure Eclipse to not auto format or to format only the changed
code, but not the whole file.
If this is not possible with Eclipse then you can use "git add -p" to
select only the functional changes in one commit and all formatting related
ones in another one.
Thanks!
On Sun, Nov 3, 2013 at 11:40 PM, mafulafunk <[email protected]> wrote:
GitHub user mafulafunk opened a pull request:
https://github.com/apache/wicket/pull/57
Assert that instance of
Ok,
this is two commits aa422c1 is just because the eclipse property
files get in the way.
The commit 0aac81f was inspired by a non informativ test fail.
Like the assert
assertTrue(factory.getFieldValue(field, obj) instanceof
ILazyInitProxy);
simply fails with no further information.
As org.hamcrest.CoreMatchers is already pulled into the classpath by
junit it might be ok to transform the given assertTrue to:
assertThat(factory.getFieldValue(field, obj),
instanceOf(ILazyInitProxy.class));
Now when the assertion fails the value of the first argument is
printed
in the test output.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mafulafunk/wicket assertThatInstanceOf
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/wicket/pull/57.patch
----
commit aa422c16a8711c43e03b65cec7148afd53153ac5
Author: Martin Funk <[email protected]>
Date: 2013-10-28T19:03:09Z
remove eclipse jdt.core and jdt.ui prefs
commit 0aac81f393047865088864c6b299ce1e022ce1fa
Author: Martin Funk <[email protected]>
Date: 2013-11-03T21:20:56Z
Refactor Testcases to make failing tests more informative:
Refactor
assertTrue(factory.getFieldValue(field, obj) instanceof
ILazyInitProxy);
to
assertThat(factory.getFieldValue(field, obj),
instanceOf(ILazyInitProxy.class));
Now when the assertion fails the value of the first argument is
printed
in the test output.
----
