-
Notifications
You must be signed in to change notification settings - Fork 649
fix(rome_cli): Use lines() separator for os independent git ignore pa… #4558
Conversation
✅ Deploy Preview for docs-rometools ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
ematipico
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! I'd ask you the following changes:
- restore the PR template and fill it
- add a changelog line, as explained in the contribution guide
- add a new test case with a gitignore file that has CRLF
|
Closes #4556 |
|
Sure, I hope this is correct? Tbh this is the first time that I pr a repo with changelog requirements, please excuse any mistakes :D |
Don't you worry, we will guide you to every step to make an awesome contribution!
Yes exactly! You can copy-paste that test, change the name of the function and the name of the snapshot - it's at the end of the function. We usually keep function name and name of the function the same. Then change |
|
Ok, I had to read about snapshot tests but thats actually pretty cool, I hope this is correct? |
ematipico
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic, thank you for your contribution! ❤️
Summary
This PR fixes parsing issues with the .gitignore files on windows. Currently the lines of the read .gitignore are separated by
\nwhich causes the parsing to break with\r\n. Thelines()method does fix this behavior.Changelog
Documentation