-
Notifications
You must be signed in to change notification settings - Fork 350
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
fix(codemod): fix tests failing due to prettier #1555
Conversation
LGTM. |
const prettierConfig = { | ||
...prettier.resolveConfig.sync(process.cwd()), | ||
parser: 'babel', | ||
endOfLine: PRETTIER_EOL |
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.
Will this work under Windows? I think this was added to support that
https://github.com/patternfly/patternfly-react/pull/1255/files#r251540310
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 don't think our project even builds on Windows currently due to path resolution problems. I'd love to support it in the future and fix things like #478
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.
To answer the question, as long as a Windows machine doesn't convert the line-endings to CRLF in JSCodemod, it will still work.
Edit: But I have no way of testing that right now :(
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.
PatternFly-React preview: https://1555-pr-patternfly-react-patternfly.surge.sh |
Codecov Report
@@ Coverage Diff @@
## master #1555 +/- ##
=========================================
Coverage ? 83.62%
=========================================
Files ? 552
Lines ? 5751
Branches ? 12
=========================================
Hits ? 4809
Misses ? 940
Partials ? 2
Continue to review full report at Codecov.
|
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.
LGTM
Will create an issue to setup a Win environment possibly to test this going forward |
What: Remove prettier EOL config option during tests. Tests fail whenever it is specified on *nix-based systems after #1255
Additional issues: