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 <[email protected]>: > 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 <[email protected]> wrote: >> On Mon, Nov 4, 2013 at 11:44 AM, Martijn Dashorst < >> [email protected]> 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 <[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/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 <[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. >>>>> >>>>> ---- >>>>> >>>>> >>> >>> >>> >>> -- >>> Become a Wicket expert, learn from the best: http://wicketinaction.com >>> > > > > -- > Become a Wicket expert, learn from the best: http://wicketinaction.com
