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

Force LF style line endings in all text files #13928

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mtreinish
Copy link
Member

Summary

As a repository we need to have consistent line endings so that we can resolve diffs sanely. Right now we haven't had an opinion on this. But some CRLF line endings caused major merge conflicts on #13278 and was preventing unraveling an otherwise simple merge conflict resolution. This commit sets a global default to use LF line endings for all text files to prevent such issues in the future.

Details and comments

The two release note files were normalized by git after adding the .gitattributes file.

As a repository we need to have consistent line endings so that we can
resolve diffs sanely. Right now we haven't had an opinion on this. But
some CRLF line endings caused major merge conflicts on Qiskit#13278 and was
preventing unraveling an otherwise simple merge conflict resolution.
This commit sets a global default to use LF line endings for all text
files to prevent such issues in the future.
@mtreinish mtreinish added the Changelog: None Do not include in changelog label Feb 26, 2025
@mtreinish mtreinish requested a review from a team as a code owner February 26, 2025 18:16
@qiskit-bot
Copy link
Collaborator

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core

@coveralls
Copy link

coveralls commented Feb 26, 2025

Pull Request Test Coverage Report for Build 13568284676

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 4 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.003%) to 87.863%

Files with Coverage Reduction New Missed Lines %
crates/accelerate/src/unitary_synthesis.rs 1 94.39%
crates/qasm2/src/lex.rs 3 91.98%
Totals Coverage Status
Change from base Build 13553732677: 0.003%
Covered Lines: 77534
Relevant Lines: 88244

💛 - Coveralls

@1ucian0
Copy link
Member

1ucian0 commented Feb 27, 2025

Somehow, it breaks the visual testing?

@jakelishman
Copy link
Member

The LaTeX and text-drawer tests use the same filecmp.cmp function as the image tests, which ofc opens the files in binary read mode, so does no line conversions. On Windows, we're writing out the files with \r\n line endings from Python space, but the references always have \n. We can either modify the .gitattributes so that test/python/visualization/references/*.{txt,tex} retains platform-native linefeeds (it's like eol=native or something), or modify the test function to pass a binary=False/binary=True flag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants