KEMBAR78
chore: add windows to CI by styfle · Pull Request #896 · vercel/ncc · GitHub
Skip to content

chore: add windows to CI #896

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
Merged
merged 10 commits into from
Mar 31, 2022
Merged

chore: add windows to CI #896

merged 10 commits into from
Mar 31, 2022

Conversation

@styfle
Copy link
Member

@styfle styfle commented Mar 25, 2022 •
edited
Loading

@styfle
Copy link
Member Author

styfle commented Mar 25, 2022

@yichi-yang Any thoughts what could be wrong here? Looks like the tests already strip \r but its still showing up

ncc/test/unit.test.js

Lines 22 to 23 in 905222e

// Windows support
.replace(/\r/g, "");

@yichi-yang
Copy link

@yichi-yang Any thoughts what could be wrong here? Looks like the tests already strip \r but its still showing up

I'm not super familiar with the unit test setup here, but just by looking at the code my guess would be it's actualSourceMap that contain \rs?

ncc/test/unit.test.js

Line 81 in 905222e

expect(actualSourceMap).toBe(expectedSourceMap);

@styfle
Copy link
Member Author

styfle commented Mar 25, 2022

That one also has a replace, so still not sure why it fails CI 🤔

ncc/test/unit.test.js

Lines 76 to 79 in 905222e

const actualSourceMap = map
.trim()
// Windows support
.replace(/\r/g, "");

@yichi-yang
Copy link

Ah I think you need /\\r/g instead of /\r/g. Those \rs in map appear to already have been escaped.

@styfle styfle marked this pull request as ready for review March 31, 2022 15:24
@styfle styfle requested a review from Timer as a code owner March 31, 2022 15:24
@styfle styfle enabled auto-merge (squash) March 31, 2022 15:24
@styfle styfle disabled auto-merge March 31, 2022 15:25
@styfle styfle merged commit 4c5fe12 into main Mar 31, 2022
@styfle styfle deleted the add-windows-ci branch March 31, 2022 15:25
@styfle styfle mentioned this pull request Jun 1, 2022
Closed
@github-actions github-actions bot mentioned this pull request May 2, 2023
Merged
@mikegorman-nf mikegorman-nf mentioned this pull request Jun 15, 2024
Open
This was referenced Jun 20, 2024
Open
Open
@TheJ-Erk400 TheJ-Erk400 mentioned this pull request Aug 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Timer Timer Awaiting requested review from Timer Timer is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@styfle @yichi-yang