-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Migrate Node tests to TypeScript #958
Conversation
*NOTE:* Had to move `node/tests` folder to `node/src/tests` since otherwise compiled JS would be in `node/lib/src` which is ugly.
@@ -59,7 +64,7 @@ const eslintConfig = | |||
} | |||
], | |||
'keyword-spacing' : 2, | |||
'linebreak-style' : [ 2, 'unix' ], | |||
'linebreak-style' : [ 2, isWindows ? 'windows' : 'unix' ], |
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.
Why would it depend on OS thought? This will mean people contributing from different operating systems having different code style that either passes Windows CI or *nix CI.
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.
This was for CI lint pass on Windows.
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.
Then it doesn't really make sense to me. Why would we want to check for different line breaks depending on OS check is running on?
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.
Pretty sure that line endings change in place from LF to CRLF when fetching the git repo on Windows. AFAIR that could be a feature of git. Probably it can be disabled.
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.
Hm, I recall something like that too... Okay, thanks for clarifying.
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.
Here: https://stackoverflow.com/questions/21822650/disable-git-eol-conversions
AFAIU it's a local setting so we cannot force the Windows machine in GH CI to behave as we would wish.
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.
I see, it is fine then, thanks!
NOTE: Had to move
node/tests
folder tonode/src/tests
since otherwise compiled JS would be innode/lib/src
which is ugly.