Skip to content
This repository has been archived by the owner on May 26, 2021. It is now read-only.

Unix line endings #173

Merged
merged 1 commit into from
Aug 11, 2017
Merged

Conversation

Pr0methean
Copy link
Contributor

Required so that Unix users can edit files without the entire tree being marked changed in a pull request that also contains real changes.

@Pr0methean
Copy link
Contributor Author

Closing since I found out about git config core.autocrlf

@Pr0methean Pr0methean closed this Aug 11, 2017
@garretwilson
Copy link
Member

The newer and better approach is .gitattributes. The old core.autocrlf approach requires that the user configure their client correctly (as you're seeing). You should read http://adaptivepatchwork.com/2012/03/01/mind-the-end-of-your-line/ .

I'm pretty sure @magnonasc went through and added a .gitattributes to https://github.com/unitsofmeasurement/uom-systems as part of unitsofmeasurement/uom-systems#66 . I recommend you do the same for the other repositories.

@Pr0methean
Copy link
Contributor Author

Pr0methean commented Aug 11, 2017

Reopening since the .gitattributes file has already been added with "* text=auto", but I guess that isn't retroactive. Anyone who wants to can use https://github.com/unitsofmeasurement/uom-se/pull/173/files?w=1 to confirm there are no real edits in this PR.

@Pr0methean Pr0methean reopened this Aug 11, 2017
@keilw
Copy link
Member

keilw commented Aug 11, 2017

This is an awful lot of files affected. I'm afraid, because many downstream projects (e.g. Eclipse SmartHome, see https://github.com/eclipse/smarthome/blob/master/targetplatform/smarthome.target) now rely on uom-se, we can only accept such PR if you're willing to sign the ECA: https://www.eclipse.org/legal/ECA.php

I talked to @garretwilson and @magnonasc about it, but their contribution (aside from a file or two here, it's primarily tests) are so far restricted to https://github.com/unitsofmeasurement/uom-systems, especially the UCUM system. Which as such is not used by Eclipse projects. UOM-SE is, so before having someone mentioned to have touched Ten Thousands lines of codes (even if it's just a linefeed setting) we must ensure, this project is not banned by Eclipse like the RI (for different reasons and given the low installation base of Java ME 8 Embedded, we can live with that for now)

Thanks,
Werner

I assume this includes #172, so if you really had a problem with the ECA, I'm afraid we could only accept relevant pieces of that PR changing only 3 files.

@magnonasc
Copy link

magnonasc commented Aug 11, 2017

Yes, it happens when you set the line endings after a lot of people using different operational systems change the files. To avoid things like this, the .gitattributes must be your template, it should be one of the first files added, so you'll never have a problem like this.

A .gitattributes must be added to this project as we've done with uom-systems, but don't forget to set the line endings into a single one, when I changed it on uom-systems it has half of its files using LF and other using CRLF, so I had to convert them into the same one (CRLF) and add the .gitattributes so it may not be affected again by any other user.

@keilw
Copy link
Member

keilw commented Aug 11, 2017

I don't think GitHub does it by default when you create a new repo, does it?

@magnonasc
Copy link

I'm not sure about that, I don't work that much with GitHub, I use mostly Bitbucket, but I don't think so.
Tell me if you find such feature here, it would be really useful.

@Pr0methean
Copy link
Contributor Author

I've now signed the ECA: https://accounts.eclipse.org/user/39818/eca

@keilw
Copy link
Member

keilw commented Aug 11, 2017

Thanks a lot. You work for Google:-) Do you also use JSR 363 there somewhere? Google is not in the JCP EC any more, but we received constructive input by Josh Bloch earlier and more recently Mark Davis.

@keilw keilw merged commit 753f9e1 into unitsofmeasurement:master Aug 11, 2017
@Pr0methean
Copy link
Contributor Author

Pr0methean commented Aug 11, 2017 via email

@keilw
Copy link
Member

keilw commented Aug 11, 2017

I did merge it. After a full build in Eclipse I was asked to push all 125 files again. Hope that did not undo anything, but as @garretwilson suggested, I added a .gitattributes file similar to those in uom-system, now it seems OK. If there is still a problem, please raise, otherwise I guess it should be OK now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants