-
1. Re: Please don't use IDE auto format on commit
timfox Jul 9, 2008 5:11 AM (in response to timfox)Here's another example of what I believe is auto format munging:
QueueImplif (status == null) { throw new IllegalStateException( "ClientConsumer.handle() should never return null"); }
I've seen quite a few examples like this throughout the code. Guys - *please* check you've turned off auto formatting! -
2. Re: Please don't use IDE auto format on commit
trustin Jul 9, 2008 5:13 AM (in response to timfox)What about automatic code cleanups like:
* trailing space removal
* Adding @Override annotation automatically
* Optimizing imports
? -
3. Re: Please don't use IDE auto format on commit
clebert.suconic Jul 9, 2008 5:14 PM (in response to timfox)This is my own opinion...
Invoking auto-format manually is okay as an exception (not every time), as long as you're 100% sure nobody else is using the same file (e.g. initial commit)... or when formatting is really screwed up for some reason.
But we should never use auto-format automatically on commit."Trustin" wrote:
* trailing space removal
This might affect SVN diffs on things you didn't change. It might cause some hassle on updates and merges"Trustin" wrote:
* Adding @Override annotation automatically
* Optimizing imports
I guess it's better to call Ctrl-Shift-O manually on Eclipse or Idea for the imports. It may change the order of imports for no reason.
I don't have an opinion about @Override. -
4. Re: Please don't use IDE auto format on commit
skajotde Jul 9, 2008 7:19 PM (in response to timfox)Eclipse 3.4 has nice option to fomat on save only last edited lines. I think auto format on edited code is ok and dont mess diffing with old code. Auto format on whole file may corrupt and reach unreadable svn diff ;)
-
5. Re: Please don't use IDE auto format on commit
trustin Jul 9, 2008 7:22 PM (in response to timfox)I agree that automatic removal of trailing spaces can be a problem for a while. However, once all trailing spaces are removed globally (e.g. a huge yet vague commit of changed empty lines), it shouldn't be a problem IMHO.
-
6. Re: Please don't use IDE auto format on commit
dmlloyd Jul 9, 2008 7:32 PM (in response to timfox)In addition to Eclipse, IDEA (and many other text editors) can support a mode wherein trailing spaces are only trimmed from modified lines. This way the file is slowly cleaned, without introducing massive sweeping code changes.
-
7. Re: Please don't use IDE auto format on commit
apwalker Jul 9, 2008 11:40 PM (in response to timfox)I find it's ok to make formatting changes but just don't do it in a commit that contains code changes. I would do it in a separate commit that only contains formatting changes with a commit message that reflects this.
just my 2cents
--Aaron -
8. Re: Please don't use IDE auto format on commit
timfox Jul 10, 2008 4:21 AM (in response to timfox)"apwalker" wrote:
I find it's ok to make formatting changes but just don't do it in a commit that contains code changes. I would do it in a separate commit that only contains formatting changes with a commit message that reflects this.
just my 2cents
--Aaron
+1.
Auto format in the same commit can make it hard to see what the original commit was about, hard to view using a diff, and all our commits should be traceable to the JIRA task which should have the JIRA change and only the JIRA change in it.
The other problem with auto formatting is everyone seems to have a different idea what "proper" formatting is. This only gets worse to standardise when everyone is using different IDEs. -
9. Re: Please don't use IDE auto format on commit
apwalker Jul 10, 2008 5:22 AM (in response to timfox)"timfox" wrote:
The other problem with auto formatting is everyone seems to have a different idea what "proper" formatting is. This only gets worse to standardise when everyone is using different IDEs.
We use checkstyle to enforce our coding standard and fail the build it doesn't conform. Also our CI system will automatically revert any change that causes a build failure.
--Aaron -
10. Re: Please don't use IDE auto format on commit
skajotde Jul 10, 2008 7:07 AM (in response to timfox)"trustin" wrote:
I agree that automatic removal of trailing spaces can be a problem for a while.
Eclipse has option to omit whitespace during comparing files from cvs. I have used this with success. When this option is enabled may be another problems? -
11. Re: Please don't use IDE auto format on commit
skajotde Jul 10, 2008 7:17 AM (in response to timfox)"timfox" wrote:
The other problem with auto formatting is everyone seems to have a different idea what "proper" formatting is. This only gets worse to standardise when everyone is using different IDEs.
I think formatting and "proper" formatting isn't so important when editing new files and check style is overuse (some formatting is required) but committing change to file cant disturbs whole file. Yes, I know I don't work on project ;) -
12. Re: Please don't use IDE auto format on commit
clebert.suconic Jul 15, 2008 5:45 PM (in response to timfox)"skajotde" wrote:
Yes, I know I don't work on project ;)
Your mistake! By participating on this forum *you are* working on the project :-P Don't be shy and you're welcome to contribute more ;-) -
13. Re: Please don't use IDE auto format on commit
skajotde Jul 17, 2008 12:06 PM (in response to timfox)"clebert.suconic@jboss.com" wrote:
"skajotde" wrote:
Yes, I know I don't work on project ;)
Your mistake! By participating on this forum *you are* working on the project :-P Don't be shy and you're welcome to contribute more ;-)
Ha ;) Personally I don't like when somebody stays on the sidelines and give advice on my project ;) Now I'm working on my masters and when finish I will think on some work on jboss and later maybe full time job ;) -
14. Re: Please don't use IDE auto format on commit
gozilla Jul 22, 2008 11:29 AM (in response to timfox)On a previous project, we had a pre-commit hook reformatting Java code according to the company standard (it was under CVS, but SVN has hooks too).
Also, the eclipse code formatter is usable from the command line.
Francois