-
Notifications
You must be signed in to change notification settings - Fork 600
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add .gitattributes file for standardized line endings #714
Conversation
This feels a little fishy to me, something that likely isn't needed and could possibly have unintended side effects. Unless someone can show a bug which exists without this change and which is fixed by this change, I would recommend not merging it. |
This is standard practice, just like .gitignore and .editorconfig files; the entire reason for adding it is to prevent unintended side effects. If a contributor clones a repo containing files with Windows line endings, makes changes locally on a Linux machine, then commits the changes, you end up with inconsistent line endings, which can cause subtle bugs in cases such as with the t4 templates. https://git-scm.com/docs/gitattributes/2.5.6 It would be prudent, on after thought, to normalize the code base prior to merging this PR, which I can take care of. |
Personally, I've only ever added a The default behavior of git is that all files are committed with |
Assuming that the user-level git configuration does not overrides this default behavior. To reiterate, there is no current issue this is fixing, as it was recently closed by #712. This is a preventative measure moving forward. I'm fairly certain that having this in place from the beginning would have prevented a lot of time and frustration for a lot of users and contributors. If you foresee a problem with introducing this to the repo based on your experience that I haven't encountered personally, I'm definitely open to reconsideration. |
I've converted this PR to a draft for the time being, pending additional feedback on this. Provided it moves forward, I'll reopen after ensuring the files are correctly normalized. |
I don't feel that strongly about it; I'm just a little wary about changing a configuration option without a good reason. And I'm not convinced that the lack of a As far as I can tell, the text files in this repo are all committed with |
This has always caused a little pain, but usually just with diff'ing when the diff logic doesn't ignore line endings. But this isn't a project concern but a user concern. With the T4 templates, however, I think from memory the line ends must be I'm happy to add a .gitattributes file. Maybe we can follow the github docs for this
|
@pleb Your comment about the |
@asherber agree. Let's leave this for now, and if it becomes an issue in the future, we address it again. The T4 templates are probably due to being retired anyway, as I'm unsure of their value over @hippasus's https://github.com/hippasus/PetaPoco.DBEntityGenerator. Plus, given we have a replacement and the fact they're super annoying to use and maintain I'd vote for their removal |
Fixes #713