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

[INFRA] dos2unix #859

Merged

Conversation

DimitriPapadopoulos
Copy link
Collaborator

I have nothing against CRLF line endings, but I am all for consistency.
This was the only text file in the whole repository with such line endings.

I have nothing against CRLF line endings, but I am all for consistency.
This was the only text file in the whole repository with such line endings.
Copy link
Collaborator

@effigies effigies left a comment

Choose a reason for hiding this comment

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

Agreed.

@DimitriPapadopoulos DimitriPapadopoulos merged commit 1ea3cbc into bids-standard:master Aug 23, 2021
@DimitriPapadopoulos
Copy link
Collaborator Author

Ouch, I had forgotten the 5 days rule. Sorry about that. Since it is a very simple INFRA change, I hope this won't matter.

@DimitriPapadopoulos DimitriPapadopoulos deleted the dos2unix branch August 23, 2021 06:37
@sappelhoff
Copy link
Member

Ouch, I had forgotten the 5 days rule. Sorry about that. Since it is a very simple INFRA change, I hope this won't matter.

indeed :-) good that you remembered it now. I agree though that for this simple INFRA change it's absolutely fine to merge after two maintainer approvals.

Thanks for the fix. If you find a simple way to extend our current CI to check for line endings (and error upon inconsistency), such a PR would be welcome.

@Remi-Gau
Copy link
Collaborator

But now it is creating a merge conflict on my "tree example" PR. 😭

Just kidding. I am sure I will survive. #Drama

@DimitriPapadopoulos
Copy link
Collaborator Author

I had a very hard time rebasing after this "dos2unix". Ended up re-creating each commit :-(

@DimitriPapadopoulos
Copy link
Collaborator Author

To fix conflicts, after git rebase master, for each conflict:

  1. keep my own version of pdf_build_src/process_markdowns.py
  2. dos2unix pdf_build_src/process_markdowns.py
  3. git add pdf_build_src/process_markdowns.py
  4. git rebase --continue

@DimitriPapadopoulos
Copy link
Collaborator Author

@effigies effigies added the exclude-from-changelog This item will not feature in the automatically generated changelog label Oct 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude-from-changelog This item will not feature in the automatically generated changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants