You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martijn Dashorst <ma...@gmail.com> on 2013/11/04 10:44:23 UTC
Autoformatting and eclipse settings
The idea is that we all have the same formatting and that it is
consistently applied to all files. Autoformatting makes that work.
That is why we have that setting in place to enforce consistent and
proper formatting without having to resort to checkstyle.
If IDEA is unable to format according to the formatting rules set in
our project perhaps there's something bonkers with the tooling?
Martijn
On Mon, Nov 4, 2013 at 9:58 AM, Martin Grigorov <mg...@apache.org> 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 <gi...@git.apache.org> 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 <ma...@gmail.com>
>> Date: 2013-10-28T19:03:09Z
>>
>> remove eclipse jdt.core and jdt.ui prefs
>>
>> commit 0aac81f393047865088864c6b299ce1e022ce1fa
>> Author: Martin Funk <ma...@gmail.com>
>> 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.
>>
>> ----
>>
>>
--
Become a Wicket expert, learn from the best: http://wicketinaction.com
Re: Autoformatting and eclipse settings
Posted by Vojtěch Krása <vo...@gmail.com>.
You must be talking about the other plugin. This one does not start Eclipse
instance.
Regards,
Vojtěch Krása
2013/11/4 Martijn Dashorst <ma...@gmail.com>
> On Mon, Nov 4, 2013 at 11:45 AM, Rusi Filipov
> <Ru...@lindenbaum.eu> wrote:
> > There is a plugin for IDEA called "Eclipse Code Formatter".
> > It can can be configured to use a formatter profile that you can export
> as .xml from Eclipse.
>
> And as I wrote in my blog post it is really a bad experience adding
> that to your IntelliJ stack. At least last time I checked it formats
> your code using an actual eclipse process (which needs to start etc.)
>
> Martijn
>
Re: Autoformatting and eclipse settings
Posted by Martijn Dashorst <ma...@gmail.com>.
On Mon, Nov 4, 2013 at 11:45 AM, Rusi Filipov
<Ru...@lindenbaum.eu> wrote:
> There is a plugin for IDEA called "Eclipse Code Formatter".
> It can can be configured to use a formatter profile that you can export as .xml from Eclipse.
And as I wrote in my blog post it is really a bad experience adding
that to your IntelliJ stack. At least last time I checked it formats
your code using an actual eclipse process (which needs to start etc.)
Martijn
AW: Autoformatting and eclipse settings
Posted by Rusi Filipov <Ru...@lindenbaum.eu>.
There is a plugin for IDEA called "Eclipse Code Formatter".
It can can be configured to use a formatter profile that you can export as .xml from Eclipse.
http://plugins.jetbrains.com/plugin/6546?pr=phpStorm
Rusi
-----Ursprüngliche Nachricht-----
Von: Martijn Dashorst [mailto:martijn.dashorst@gmail.com]
Gesendet: Monday, November 04, 2013 10:44
An: dev@wicket.apache.org
Betreff: Autoformatting and eclipse settings
The idea is that we all have the same formatting and that it is consistently applied to all files. Autoformatting makes that work.
That is why we have that setting in place to enforce consistent and proper formatting without having to resort to checkstyle.
If IDEA is unable to format according to the formatting rules set in our project perhaps there's something bonkers with the tooling?
Martijn
On Mon, Nov 4, 2013 at 9:58 AM, Martin Grigorov <mg...@apache.org> 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/0aac81f393047865088864c6b2
> 99ce1e022ce1fa
> (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 <gi...@git.apache.org> 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 <ma...@gmail.com>
>> Date: 2013-10-28T19:03:09Z
>>
>> remove eclipse jdt.core and jdt.ui prefs
>>
>> commit 0aac81f393047865088864c6b299ce1e022ce1fa
>> Author: Martin Funk <ma...@gmail.com>
>> 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.
>>
>> ----
>>
>>
--
Become a Wicket expert, learn from the best: http://wicketinaction.com
Re: Autoformatting and eclipse settings
Posted by Martin Funk <ma...@gmail.com>.
Sorry that this tends to a code-formatting subject. That's far of my intention.
I just started form the build wicket tutorial at http://wicket.apache.org/contribute/build.html
which leaves eclipse users with a bunch of modified eclipse .settings files in the repo.
Not the prettiest sight as an entry point for someone (eclipse user) trying to find an approach
to the wicket code.
So basically it is an eclipse and its .settings file problem. I think as long as m2e,
maven-eclipse-plugin and eclipse itself with its different incarnations are around, they
will mess around in those settings files. Eclipse itself might not mind that much,
but m2e and maven-eclipse-plugin don't get along with each other that well.
But as I see it those files are not needed to bootstrap an eclipse project.
Both m2e and maven-eclipse-plugin can generate the .project and .classpath files as
well as mess around in the .settings files.
So I'd sugest to drop those eclipse files out of the repo all together. If I interpret the
git annotations of the .gitignore file correctly, Michael Mosmann might have had the
same idea. If a new module is created no .settings files will be committed.
The existing .settings files are just not dropped because they were in the repo before
the entry "**.settings" was introduced to the .gitignore file.
To drop them they explicitly need to be delete using "git rm"
The code formatting question is independent to this. The suggestion I'd give to eclipse
users is to import the EclipsCodeFormatting.xml.
Those suggestions I wrote down in the attachment of https://issues.apache.org/jira/browse/WICKET-5399
Basically the lines I edited in contribute/build.md.
Beyond what I wrote there eclipse users could be hinted to the eclipse safe format action:
"Format edited lines : If selected, only the lines that have been modified since the last save are formatted on save."
:-)
Martin
Am 04.11.2013 um 14:11 schrieb Martijn Dashorst <ma...@gmail.com>:
> Often eclipse adds new formatting settings that we (eclipse users)
> don't know about either. I guess most options are available in the
> settings UI, but I must admit I don't go through them and look at what
> is different between one version and another. I do read the New and
> Noteworthy sections of the releases but it seems the Java environment
> is dead in the water, and new formatting rules either don't register
> with me or are not considered N&N.
>
> I had hoped that the company behind IntelliJ would get its act
> together and have a proper importer for eclipse settings making the
> transition from eclipse to IntelliJ Idea a much better experience.
> Alas they are not...
>
>> But in any case, I think Eclipse should not auto-format the whole file. It
>> should format only the lines where the developer makes his changes. If the
>> developer wants to format the whole file (or even codebase) he should do it
>> in a separate commit.
>
> If the settings are correct and interpreted correctly by the IDE
> (Eclipse, Netbeans and IntelliJ) then auto-reformatting the whole file
> won't matter since the code would be properly formatted beforehand.
>
> That said, perhaps we should consider adopting a new formatting
> standard (Sun?) and format all active branches accordingly after
> ensuring that all IDE's format consistently. Though I don't enjoy
> starting a new discussion on tabs vs spaces.
>
> Martijn
>
>
> On Mon, Nov 4, 2013 at 10:57 AM, Martin Grigorov <mg...@apache.org> wrote:
>> On Mon, Nov 4, 2013 at 11:44 AM, Martijn Dashorst <
>> martijn.dashorst@gmail.com> wrote:
>>
>>> The idea is that we all have the same formatting and that it is
>>> consistently applied to all files. Autoformatting makes that work.
>>> That is why we have that setting in place to enforce consistent and
>>> proper formatting without having to resort to checkstyle.
>>>
>>> If IDEA is unable to format according to the formatting rules set in
>>> our project perhaps there's something bonkers with the tooling?
>>>
>>
>> You are partly right.
>> My tooling is at
>> https://github.com/martin-g/dotfiles/blob/master/.IntelliJIdea11/config/codestyles/wicket.xml
>> I have configured IDEA to follow the committed Eclipse settings.
>> But from time to time I see that there are commits in .settings/ without
>> explanation what is the change. Most probably a new version of Eclipse or
>> any of its plugins changed something internal.
>> If the developer who changed something in .settings/ describe the change in
>> the commit message then I apply the change in my IDEA config.
>>
>> The problem is not IDEA. The problem is with different versions of Eclipse
>> and its plugins. Emond had explained the same problem at
>> http://markmail.org/search/?q=list%3Aorg.apache.wicket+org.eclipse.jdt.core.prefs#query:list%3Aorg.apache.wicket%20org.eclipse.jdt.core.prefs%20order%3Adate-backward+page:1+mid:uue2s7acqs3h4zuo+state:results
>>
>> Martin Funk is an Eclipse user and he says that just importing Wicket in
>> his Eclipse leads to Eclipse making changes in many files in .settings/.
>>
>> But in any case, I think Eclipse should not auto-format the whole file. It
>> should format only the lines where the developer makes his changes. If the
>> developer wants to format the whole file (or even codebase) he should do it
>> in a separate commit.
>> For example
>> https://github.com/mafulafunk/wicket/commit/0aac81f393047865088864c6b299ce1e022ce1fa
>> should
>> not have the second and the third diff chunks in this commit.
>>
>>
>>>
>>> Martijn
>>>
>>> On Mon, Nov 4, 2013 at 9:58 AM, Martin Grigorov <mg...@apache.org>
>>> 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 <gi...@git.apache.org> 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/wicketassertThatInstanceOf
>>>>>
>>>>> 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 <ma...@gmail.com>
>>>>> Date: 2013-10-28T19:03:09Z
>>>>>
>>>>> remove eclipse jdt.core and jdt.ui prefs
>>>>>
>>>>> commit 0aac81f393047865088864c6b299ce1e022ce1fa
>>>>> Author: Martin Funk <ma...@gmail.com>
>>>>> 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.
>>>>>
>>>>> ----
>>>>>
>>>>>
>>>
>>>
>>>
>>> --
>>> Become a Wicket expert, learn from the best: http://wicketinaction.com
>>>
>
>
>
> --
> Become a Wicket expert, learn from the best: http://wicketinaction.com
Re: Autoformatting and eclipse settings
Posted by Martijn Dashorst <ma...@gmail.com>.
Often eclipse adds new formatting settings that we (eclipse users)
don't know about either. I guess most options are available in the
settings UI, but I must admit I don't go through them and look at what
is different between one version and another. I do read the New and
Noteworthy sections of the releases but it seems the Java environment
is dead in the water, and new formatting rules either don't register
with me or are not considered N&N.
I had hoped that the company behind IntelliJ would get its act
together and have a proper importer for eclipse settings making the
transition from eclipse to IntelliJ Idea a much better experience.
Alas they are not...
> But in any case, I think Eclipse should not auto-format the whole file. It
> should format only the lines where the developer makes his changes. If the
> developer wants to format the whole file (or even codebase) he should do it
> in a separate commit.
If the settings are correct and interpreted correctly by the IDE
(Eclipse, Netbeans and IntelliJ) then auto-reformatting the whole file
won't matter since the code would be properly formatted beforehand.
That said, perhaps we should consider adopting a new formatting
standard (Sun?) and format all active branches accordingly after
ensuring that all IDE's format consistently. Though I don't enjoy
starting a new discussion on tabs vs spaces.
Martijn
On Mon, Nov 4, 2013 at 10:57 AM, Martin Grigorov <mg...@apache.org> wrote:
> On Mon, Nov 4, 2013 at 11:44 AM, Martijn Dashorst <
> martijn.dashorst@gmail.com> wrote:
>
>> The idea is that we all have the same formatting and that it is
>> consistently applied to all files. Autoformatting makes that work.
>> That is why we have that setting in place to enforce consistent and
>> proper formatting without having to resort to checkstyle.
>>
>> If IDEA is unable to format according to the formatting rules set in
>> our project perhaps there's something bonkers with the tooling?
>>
>
> You are partly right.
> My tooling is at
> https://github.com/martin-g/dotfiles/blob/master/.IntelliJIdea11/config/codestyles/wicket.xml
> I have configured IDEA to follow the committed Eclipse settings.
> But from time to time I see that there are commits in .settings/ without
> explanation what is the change. Most probably a new version of Eclipse or
> any of its plugins changed something internal.
> If the developer who changed something in .settings/ describe the change in
> the commit message then I apply the change in my IDEA config.
>
> The problem is not IDEA. The problem is with different versions of Eclipse
> and its plugins. Emond had explained the same problem at
> http://markmail.org/search/?q=list%3Aorg.apache.wicket+org.eclipse.jdt.core.prefs#query:list%3Aorg.apache.wicket%20org.eclipse.jdt.core.prefs%20order%3Adate-backward+page:1+mid:uue2s7acqs3h4zuo+state:results
>
> Martin Funk is an Eclipse user and he says that just importing Wicket in
> his Eclipse leads to Eclipse making changes in many files in .settings/.
>
> But in any case, I think Eclipse should not auto-format the whole file. It
> should format only the lines where the developer makes his changes. If the
> developer wants to format the whole file (or even codebase) he should do it
> in a separate commit.
> For example
> https://github.com/mafulafunk/wicket/commit/0aac81f393047865088864c6b299ce1e022ce1fa
> should
> not have the second and the third diff chunks in this commit.
>
>
>>
>> Martijn
>>
>> On Mon, Nov 4, 2013 at 9:58 AM, Martin Grigorov <mg...@apache.org>
>> 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 <gi...@git.apache.org> 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/wicketassertThatInstanceOf
>> >>
>> >> 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 <ma...@gmail.com>
>> >> Date: 2013-10-28T19:03:09Z
>> >>
>> >> remove eclipse jdt.core and jdt.ui prefs
>> >>
>> >> commit 0aac81f393047865088864c6b299ce1e022ce1fa
>> >> Author: Martin Funk <ma...@gmail.com>
>> >> 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.
>> >>
>> >> ----
>> >>
>> >>
>>
>>
>>
>> --
>> Become a Wicket expert, learn from the best: http://wicketinaction.com
>>
--
Become a Wicket expert, learn from the best: http://wicketinaction.com
Re: Autoformatting and eclipse settings
Posted by Martin Grigorov <mg...@apache.org>.
On Mon, Nov 4, 2013 at 11:44 AM, Martijn Dashorst <
martijn.dashorst@gmail.com> wrote:
> The idea is that we all have the same formatting and that it is
> consistently applied to all files. Autoformatting makes that work.
> That is why we have that setting in place to enforce consistent and
> proper formatting without having to resort to checkstyle.
>
> If IDEA is unable to format according to the formatting rules set in
> our project perhaps there's something bonkers with the tooling?
>
You are partly right.
My tooling is at
https://github.com/martin-g/dotfiles/blob/master/.IntelliJIdea11/config/codestyles/wicket.xml
I have configured IDEA to follow the committed Eclipse settings.
But from time to time I see that there are commits in .settings/ without
explanation what is the change. Most probably a new version of Eclipse or
any of its plugins changed something internal.
If the developer who changed something in .settings/ describe the change in
the commit message then I apply the change in my IDEA config.
The problem is not IDEA. The problem is with different versions of Eclipse
and its plugins. Emond had explained the same problem at
http://markmail.org/search/?q=list%3Aorg.apache.wicket+org.eclipse.jdt.core.prefs#query:list%3Aorg.apache.wicket%20org.eclipse.jdt.core.prefs%20order%3Adate-backward+page:1+mid:uue2s7acqs3h4zuo+state:results
Martin Funk is an Eclipse user and he says that just importing Wicket in
his Eclipse leads to Eclipse making changes in many files in .settings/.
But in any case, I think Eclipse should not auto-format the whole file. It
should format only the lines where the developer makes his changes. If the
developer wants to format the whole file (or even codebase) he should do it
in a separate commit.
For example
https://github.com/mafulafunk/wicket/commit/0aac81f393047865088864c6b299ce1e022ce1fa
should
not have the second and the third diff chunks in this commit.
>
> Martijn
>
> On Mon, Nov 4, 2013 at 9:58 AM, Martin Grigorov <mg...@apache.org>
> 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 <gi...@git.apache.org> 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/wicketassertThatInstanceOf
> >>
> >> 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 <ma...@gmail.com>
> >> Date: 2013-10-28T19:03:09Z
> >>
> >> remove eclipse jdt.core and jdt.ui prefs
> >>
> >> commit 0aac81f393047865088864c6b299ce1e022ce1fa
> >> Author: Martin Funk <ma...@gmail.com>
> >> 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.
> >>
> >> ----
> >>
> >>
>
>
>
> --
> Become a Wicket expert, learn from the best: http://wicketinaction.com
>
Re: Autoformatting and eclipse settings
Posted by Nick Pratt <nb...@gmail.com>.
Is this worth the effort?
Why dont we just have a re-format of the entire code base as a single
checkin prior to every release. Obviously everyone should try to format as
consistently as possible, and given the fact that there are some
discrepancies between Eclipse and IntelliJ in formatting, if there are
obvious ones, why don't we try and clarify them and either adjust the
Eclipse rules a little to accommodate, or log bugs with Jetbrains to see if
they will fix them - they are pretty responsive in this regard.
In my experience you can get the styles pretty darned close, and either
editor can handle merges (which is the real issue with formatting IMHO) on
slightly different formats very well.
On Mon, Nov 4, 2013 at 12:18 PM, Martijn Dashorst <
martijn.dashorst@gmail.com> wrote:
> On Mon, Nov 4, 2013 at 1:01 PM, Martin Grigorov <mg...@apache.org>
> 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.
>
> Not quite the correct characterization of my intention.
>
> I merely pointed out the reason why we enforced autoformat on save in
> our settings. And I still think that option is a good one (it
> certainly makes our company's code consistent across the board with
> few merge conflicts due to formatting inconsistencies).
>
> I sure think everybody should use the tool they love. And I have great
> sympathy for IntelliJ, but I find their tooling lacking in supporting
> migrations from competing IDEs [1]. I would also like that anybody
> using Netbeans could contribute to our project without any hassle.
>
> The fact is that we have codified our styling guidelines in Eclipse
> formatting rules and a specific Eclipse formatting xml (which probably
> is hopelessly out of date–will look into that). If one chooses to use
> another IDE, I'd hope that the precise formatting rules are replicated
> into that new environment such that a reformat of the code base will
> produce the same results as a reformat in Eclipse would do.
>
> Now I suspect that doing a reformat using Eclipse on our code base
> will result in many updated files, just because of commit rot
> (imported patches, one-off commandline vi fixes, etc).
>
> So:
>
> 1. our code formatting guidelines should be codified
> 2. all our IDEs should adhere to these code formatting rules
> 3. they should generate exact the same files
> 4. autoformat should not produce modified lines in parts that are unchanged
> 5. ideally we should not have IDE specific files in our repository
>
> (3 follows from 2, and 4 follows from 3, but I think making them
> explicit makes the intention clear).
>
> ad 5: at my company we have a resource bundle that includes the
> companywide project settings, which get imported by the eclipse plugin
> (not sure if m2e picks it up though)
>
> Now: who's up to help ensure 1. is consistent, correct and in
> accordance with our expectations? In what format should we codify our
> formatting rules? Eclipse formatting? Checkstyle [2]?
>
> Martijn
>
> [1]
> http://martijndashorst.com/blog/2013/11/04/intellij-not-the-best-for-me/
> [2]
> http://stackoverflow.com/questions/984778/how-to-generate-an-eclipse-formatter-configuration-from-a-checkstyle-configurati
>
>
> On Mon, Nov 4, 2013 at 10:44 AM, Martijn Dashorst
> <ma...@gmail.com> wrote:
> > The idea is that we all have the same formatting and that it is
> > consistently applied to all files. Autoformatting makes that work.
> > That is why we have that setting in place to enforce consistent and
> > proper formatting without having to resort to checkstyle.
> >
> > If IDEA is unable to format according to the formatting rules set in
> > our project perhaps there's something bonkers with the tooling?
> >
> > Martijn
> >
> > On Mon, Nov 4, 2013 at 9:58 AM, Martin Grigorov <mg...@apache.org>
> 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 <gi...@git.apache.org> 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/wicketassertThatInstanceOf
> >>>
> >>> 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 <ma...@gmail.com>
> >>> Date: 2013-10-28T19:03:09Z
> >>>
> >>> remove eclipse jdt.core and jdt.ui prefs
> >>>
> >>> commit 0aac81f393047865088864c6b299ce1e022ce1fa
> >>> Author: Martin Funk <ma...@gmail.com>
> >>> 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.
> >>>
> >>> ----
> >>>
> >>>
> >
> >
> >
> > --
> > Become a Wicket expert, learn from the best: http://wicketinaction.com
>
>
>
> --
> Become a Wicket expert, learn from the best: http://wicketinaction.com
>
Re: Autoformatting and eclipse settings
Posted by Martijn Dashorst <ma...@gmail.com>.
On Mon, Nov 4, 2013 at 1:01 PM, Martin Grigorov <mg...@apache.org> 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.
Not quite the correct characterization of my intention.
I merely pointed out the reason why we enforced autoformat on save in
our settings. And I still think that option is a good one (it
certainly makes our company's code consistent across the board with
few merge conflicts due to formatting inconsistencies).
I sure think everybody should use the tool they love. And I have great
sympathy for IntelliJ, but I find their tooling lacking in supporting
migrations from competing IDEs [1]. I would also like that anybody
using Netbeans could contribute to our project without any hassle.
The fact is that we have codified our styling guidelines in Eclipse
formatting rules and a specific Eclipse formatting xml (which probably
is hopelessly out of date–will look into that). If one chooses to use
another IDE, I'd hope that the precise formatting rules are replicated
into that new environment such that a reformat of the code base will
produce the same results as a reformat in Eclipse would do.
Now I suspect that doing a reformat using Eclipse on our code base
will result in many updated files, just because of commit rot
(imported patches, one-off commandline vi fixes, etc).
So:
1. our code formatting guidelines should be codified
2. all our IDEs should adhere to these code formatting rules
3. they should generate exact the same files
4. autoformat should not produce modified lines in parts that are unchanged
5. ideally we should not have IDE specific files in our repository
(3 follows from 2, and 4 follows from 3, but I think making them
explicit makes the intention clear).
ad 5: at my company we have a resource bundle that includes the
companywide project settings, which get imported by the eclipse plugin
(not sure if m2e picks it up though)
Now: who's up to help ensure 1. is consistent, correct and in
accordance with our expectations? In what format should we codify our
formatting rules? Eclipse formatting? Checkstyle [2]?
Martijn
[1] http://martijndashorst.com/blog/2013/11/04/intellij-not-the-best-for-me/
[2] http://stackoverflow.com/questions/984778/how-to-generate-an-eclipse-formatter-configuration-from-a-checkstyle-configurati
On Mon, Nov 4, 2013 at 10:44 AM, Martijn Dashorst
<ma...@gmail.com> wrote:
> The idea is that we all have the same formatting and that it is
> consistently applied to all files. Autoformatting makes that work.
> That is why we have that setting in place to enforce consistent and
> proper formatting without having to resort to checkstyle.
>
> If IDEA is unable to format according to the formatting rules set in
> our project perhaps there's something bonkers with the tooling?
>
> Martijn
>
> On Mon, Nov 4, 2013 at 9:58 AM, Martin Grigorov <mg...@apache.org> 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 <gi...@git.apache.org> 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 <ma...@gmail.com>
>>> Date: 2013-10-28T19:03:09Z
>>>
>>> remove eclipse jdt.core and jdt.ui prefs
>>>
>>> commit 0aac81f393047865088864c6b299ce1e022ce1fa
>>> Author: Martin Funk <ma...@gmail.com>
>>> 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.
>>>
>>> ----
>>>
>>>
>
>
>
> --
> Become a Wicket expert, learn from the best: http://wicketinaction.com
--
Become a Wicket expert, learn from the best: http://wicketinaction.com