On Mon, Nov 4, 2013 at 1:01 PM, Martin Grigorov <[email protected]> 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 <[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? > > 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/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. >>> >>> ---- >>> >>> > > > > -- > Become a Wicket expert, learn from the best: http://wicketinaction.com -- Become a Wicket expert, learn from the best: http://wicketinaction.com
