-
Notifications
You must be signed in to change notification settings - Fork 6k
Fixed errors introduced in landing of fml changes for Windows. #4431
Conversation
chinmaygarde
left a comment
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.
We should come up with a more maintainable solution to window compiler oddities. I dont think one off preprocessor guards are the way to go.
| for (size_t i = 0; i < count; i++) { | ||
| loop.GetTaskRunner()->PostTask([&terminated, i, ¤t, count]() { | ||
| loop.GetTaskRunner()->PostTask([&terminated, i, ¤t | ||
| #if OS_WIN |
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 have no idea what issue this is trying to work around on Windows. This is not maintainable and I am sure the next time anyone tries to modify such TUs, he/she is going to miss one of these preprocessor guards.
|
Also, please get PRs reviewed before landing. Adding a reviewer to the PR usually gets that persons attention pretty quickly. |
|
@chinmaygarde my bad, I should have submitted for review but was trying to get the bots green. The change above is because MSVC requires |
No description provided.