-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Fix Build Errors on Windows #9726
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
Conversation
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.
Thanks @marcusdacoregio! Can you please add a test for us. I think the test is updating the GitHub actions to run ./gradlew check on Windows.
2426c4d to
0586269
Compare
|
Thank you for your review @rwinch . I've added the test in the GitHub actions to perform the |
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.
Thanks, @marcusdacoregio! Please see my feedback inline.
Additionally, I think the commits should be like this:
- one commit for adding
.gitattributes - one commit for changing the CI workflow
- one commit for updating
buildSrcto work on Windows
Then it will be simpler to backport the .gitattributes changes.
|
Thanks for your review @jzheaux . I updated the solution with your considerations. Let me know if there is something else that we can improve. |
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.
Thanks for the updates, @marcusdacoregio. If you aren't anticipating any further changes, will you please also rebase and change check_windows to include the dependency on the prerequisites task that was added in #9701?
6d781ac to
ad6936c
Compare
Normalize line endings to auto Ensure that some file extensions are treated as binary
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.
Leaving the rest of the feedback to @jzheaux
|
Thanks, @marcusdacoregio! This is now merged into I did a polish (02474fd) to add some additional binary files to |
Closes gh-9724, gh-9727