Skip to content

Conversation

@rochdev
Copy link
Member

@rochdev rochdev commented Aug 4, 2021

What does this PR do?

Add tests in CI for Windows and macOS to officialize support for them.

Motivation

Windows tests are failing in dd-trace because of a transitive dependency of sketches-js, indicating a potential issue with Windows support.

@rochdev rochdev force-pushed the rochdev/windows-and-mac-support branch from 2600067 to fc2cce4 Compare August 5, 2021 15:05
@rochdev rochdev marked this pull request as ready for review August 5, 2021 16:39
@rochdev rochdev requested a review from brimtown August 5, 2021 16:39
'plugin:prettier/recommended'
]
],
rules: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we want this here. This prettier/prettier ESLint plugin is reading the settings from the prettier.config.js file, so any formatting-config changes we'd make should go in prettier.config.js

I'm guessing that if you ran into an issue with this, instead of swapping the default value for endOfLine, we should set up a pre-commit hook to run prettier with the project's configuration (or make sure that your local editor is set up to run prettier itself).

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that Windows uses CRLF, and by default Prettier enforces LF only. So the build fails on Windows because of this limitation. I can move the rule to prettier.config.js, but the rule is still needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

@brimtown When I add a Prettier config file, it looks line the ESLint config file starts to be ignored. From the information I found online, I think when both Prettier and ESLint are combined then any Prettier configuration should be done in the ESLint configuration. Please let me know if you know of a way to use both.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants