Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [eclipselink-dev] Unification of line endings in the codebase

On 3/26/15 2:26 PM, andrei ilitchev wrote:
After formatting sweep it would be impossible (well, much harder than before) to follow revision history for real changes.

agree with this point. OTOH I think it can be made "less harder" (this does not sound like right english term to me :-)) if changes are made all at once, it will become clear what exactly to skip in the history

there will have to be at least two commits in this case anyway - one for line endings change, the other for the rest of stuff (whitespaces, cp year, formatting(?))
Also formatting rules are hard to enforce, especially after the initial enthusiasm is gone ...

true, rule which is not being enforced does not make much sense to me neither. Enforcement on the server side (by rejecting pushes not following given conventions) is possible, even though not for everything probably but that can be too late. I was thinking about providing also some client side git pre-commit hooks which would be able to check basic stuff (whitespaces, LF, formatting(?)) during 'git commit' with hard check for proper setup of those hooks on developers machine during regular build. Sure there will always be a way how to bypass such hooks but it should be used in rare cases only.

What do you or others think about these client-side hooks? Would you find them useful?

Thanks,
--lukas
On 3/26/2015 9:09 AM, Rick Curtis wrote:
> such as replace tabs for spaces (many files still use tabs), or even apply single formatting rules to all files (many files have broken formatting, non-consistent formatting is used across the codebase), so that we could potentialy integrate checkstyle to make sure we keep the status going forward. Updating to 2015 would be fine, too.

+1

I'd like to see checkstyle run every time we build. It's sometimes a pain, but the best way to ensure that crud doesn't creep into our code base.

On Thu, Mar 26, 2015 at 7:59 AM, Lukas Jungmann <lukas.jungmann@xxxxxxxxxx> wrote:
On 3/25/15 9:26 AM, Martin Grebac wrote:
The main purpose of this is to make reviews easier and code easier to work with, so even though line endings solution would help tremendously if we're doing the mass update I'd be voting for broadening the scope to also apply other formatting - such as replace tabs for spaces (many files still use tabs),

ok, replacing tabs for spaces together with trimming leading whitespaces can be done

or even apply single formatting rules to all files (many files have broken formatting, non-consistent formatting is used across the codebase), so that we could potentialy integrate checkstyle to make sure we keep the status going forward.

I haven't found simple way to convert existing eclipse rules, pointed out by Gordon, into checkstyle ones. Not sure how complicated that would be. OTOH there does not seem to be simple way how we could enforce those rules to be followed other then providing client side git pre-commit hook, ie by adopting sth like https://gist.github.com/davetron5000/37350, which would have to be set up manually by each developer on his local machine

Updating to 2015 would be fine, too.

ok, will do this as well.

thanks,
--lukas



Opinions?

 MartiNG 

On 19.03.15 10:21, Lukas Jungmann wrote:
On 3/18/15 3:40 PM, David Minsky wrote:
+LF

Also consider: any plans to change the date in the copyright headers to 2015 at the same time, since a mass update is being planned for that specific day?

I'm not big fan of these mass updates just because of year has changed, I think it is better to update what actually changed only. But since I'll be touching a lot of files, it perhaps make sense to update the year as well... will think about this

thanks,
--lukas


On 18/03/2015 4:48 AM, Lukas Jungmann wrote:
Hi all,

   currently eclipselink sources contains mix of line endings - some files are using *nix style (LF), some win style (CRLF) and the rest is a mix of *nix and win style (some lines end with LF while others with CRLF). Since we are at the beginning of 2.7 development it looks like a good time to me to unify line endings in our codebase to avoid complete rewrites of files in the future.

Why we should do that?
-make it easier to review incoming as well as own (outgoing) changes
-avoid noise in git log
-use https://help.github.com/articles/dealing-with-line-endings as a guide to properly set up local development environment

Drawbacks:
-one-time full rewrite of some files in the repository
-some backports to previous streams will require manual changes/merges

When?
  if there will be an agreement, I'd like to perform the change next week, on Friday March 27, 2015

Which line endings should be used?
  either *nix or win but definitely not a mix of them in one file

How to cast your vote?
  simply reply to this email with either '+LF', '+CRLF' or 'Leave it as it is' by Thursday March 28, 23:59.

Thanks and happy voting,
--lukas
_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev


--
Oracle
David Minsky | Principal Member of Technical Staff | +1 613-288-4605
Oracle Sustaining Engineering | Application Server
45 O'Connor Street, Suite 400, Ottawa, Ontario, Canada, K1P 1A4


_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev



_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

-- 
Martin Grebac, SW Engineering Manager
Oracle Czech, Prague


_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev


_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev



--
Rick Curtis


_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev



_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev


Back to the top