Skip to content
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

refactor: Renormalize to ensure consistent file line termination. #226

Merged

Conversation

shahzadlone
Copy link
Member

@shahzadlone shahzadlone commented Feb 23, 2022

ISSUE:

Resolves #225

DESCRIPTION:

Git diff shows added lines and removed lines that are exactly the same except (LF or CRLF line endings, which aren't even visible). This makes reviewing diffs very painful, specially linter refactoring that modifies lots of lines, the files show up as if the line from each file was removed and then added (can't really compare properly).
Hence this solution, with this config-based approach, we can ensure that our line endings remain consistent throughout our code-base regardless of what operating systems or local Git settings the developers use.
Beauty with this approach is that Windows and Linux users locally will keep what they want to use, when checked in they will consistently have LF line endings.
Note: We also ensures that binary files are not modified.

REFERENCE:

https://www.aleksandrhovhannisyan.com/blog/crlf-vs-lf-normalizing-line-endings-in-git/

Before PR:

image

After PR:

image

@shahzadlone shahzadlone self-assigned this Feb 23, 2022
@shahzadlone shahzadlone added code quality Related to improving code quality refactor This issue specific to or requires *notable* refactoring of existing codebases and components labels Feb 23, 2022
@shahzadlone shahzadlone added this to the DefraDB v0.3 milestone Feb 23, 2022
Copy link
Contributor

@orpheuslummis orpheuslummis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple & automated, so looking fine.

@AndrewSisley
Copy link
Contributor

see my comments in core-dev. Really prefer we dont do this now

@shahzadlone shahzadlone force-pushed the lone/refactor/i-225/normalize-consistent-line-endings branch from d9c7c8f to b3194d7 Compare March 4, 2022 23:36
@codecov
Copy link

codecov bot commented Mar 4, 2022

Codecov Report

Merging #226 (b3194d7) into develop (6bfcf7e) will not change coverage.
The diff coverage is 46.43%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #226   +/-   ##
========================================
  Coverage    58.12%   58.12%           
========================================
  Files          103      103           
  Lines        10224    10224           
========================================
  Hits          5943     5943           
  Misses        3647     3647           
  Partials       634      634           
Impacted Files Coverage Δ
api/http/api.go 0.00% <0.00%> (ø)
bench/collection/utils.go 0.00% <0.00%> (ø)
bench/fixtures/fixtures.go 0.00% <0.00%> (ø)
bench/query/planner/utils.go 0.00% <0.00%> (ø)
bench/query/simple/utils.go 0.00% <0.00%> (ø)
bench/storage/utils.go 0.00% <0.00%> (ø)
db/api.go 0.00% <0.00%> (ø)
db/blockstore.go 0.00% <0.00%> (ø)
document/document.go 63.95% <ø> (ø)
document/encoded.go 65.59% <ø> (ø)
... and 63 more

Copy link
Member

@jsimnz jsimnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After meticulously reviewing all 54,000 add/deletes, I give this 2 👍 up.

Actually I just ran a local renormalize command and made sure it matched yours. :)

@shahzadlone shahzadlone merged commit f0bac01 into develop Mar 7, 2022
@shahzadlone shahzadlone deleted the lone/refactor/i-225/normalize-consistent-line-endings branch March 7, 2022 03:52
shahzadlone added a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Related to improving code quality refactor This issue specific to or requires *notable* refactoring of existing codebases and components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Normalize existing files and ensure we use LF
4 participants