monarch-initiative / mondo-ingest

Coordinating the mondo-ingest with external sources
https://monarch-initiative.github.io/mondo-ingest/
6 stars 3 forks source link

Benign attempt to resolve strange `LF` --> `CRLF` issue on Joe's machine #630

Open joeflack4 opened 3 months ago

joeflack4 commented 3 months ago

Overview

Add .gitattributes to .gitignore
This allows individual contributors to add a .gitattributes for their needs, but opts for individualized .gitattributes per contributor.
Use case: One contributor is having an issue where, despite global git configuration, a lot of the line endings for files are getting converted to CRLF (Windows) rather than LF (Unix). The .gitattributes solution attempts to solve this, but since others aren't having this issue, it may be better to not bloat the repo by adding a file that it doesn't generally need.

Pre-merge checklist

Documentation

Was the documentation added/updated under docs/?

QC

Was the full pipeline run before submitting this PR using sh run.sh make build-mondo-ingest on this branch (after docker pull obolibrary/odkfull:dev), and no errors occurred?

New Packages

Were any new Python packages added?

Were any other non-Python packages added?

PR Review and Conversations Resolved

Has the PR been sufficiently reviewed by at least 1 team member of the Mondo Technical team and all threads resolved?

joeflack4 commented 3 months ago

Will bring this up at next 1:1 meeting.

twhetzel commented 3 months ago

@joeflack4 do you have a follow-up commit somewhere to show the before and after effect of using this to specify the end of line issue you're encountering?

joeflack4 commented 3 months ago

I expect no before/after effect on the git history. If it actually helps, I should only notice the effect on on my end.

I still don't understand why this is happening all of a sudden and only in some of my repos, why it doesn't listen to my global configs, and also sometimes when I pushed, I did see line end differences, but I haven't seen that for a while. If I commit and I do see differences, I'll revert and abandon this .gitattributes solution.

matentzn commented 3 months ago

After reflecting on this:

While it is uncommon to do this, this allows @joeflack4 to configure his repo locally in whatever way he wants. It does not affect the other team members. The only reason to not merge this is if we believe that we have to figure out the root cause of the issue, but this is the plaster that stops the bleeding and stopping the bleeding is good enough, I think we should merge this and move on. It is indeed a totally benign change for other team members!