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.

Sign up for GitHub

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

Jump to bottom

Replace BS.init with T.strip to handle windows newlines (\r\n) #2095

Merged
merged 1 commit into from
Aug 31, 2020
Merged

Replace BS.init with T.strip to handle windows newlines (\r\n) #2095

merged 1 commit into from
Aug 31, 2020

Conversation

Anviking
Copy link
Collaborator

Issue Number

#2083 / related

Overview

  • Replace BS.init with T.strip to handle windows newlines (\r\n). Integration tests failed with a pattern match failure here, when I ran locally on windows.

Comments

  • I've never seen CI fail for this reason, which is arguably strange.

@Anviking Anviking self-assigned this Aug 31, 2020
KtorZ
KtorZ approved these changes Aug 31, 2020
View reviewed changes
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

Subtle... There are maybe other places in the Launch code where we simply assume unix newlines...

@Anviking
Copy link
Collaborator Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 31, 2020

Build succeeded

@iohk-bors iohk-bors bot merged commit 2b76b50 into master Aug 31, 2020
@iohk-bors iohk-bors bot deleted the anviking/2083/windows-newlines branch August 31, 2020 17:42
@rvl
Copy link
Contributor

rvl commented Aug 31, 2020

The reason only you can see the error is because your git on windows is configured to automatically convert newlines when checking out files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KtorZ KtorZ KtorZ approved these changes

Assignees

@Anviking Anviking

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Anviking @rvl @KtorZ