Skip to content
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

Win32 COFF: fix std.process unittests #3083

Merged
merged 1 commit into from
Mar 24, 2015

Conversation

rainers
Copy link
Member

@rainers rainers commented Mar 20, 2015

use CRuntime_Microsoft instead of Win64.

@quickfur
Copy link
Member

Does nobody care about Windows support?! It's been 4 days and not a single review...

Anyway, I'm not exactly qualified to review this either since I don't have a Windows dev environment, but the autotester has been green for a while now and I'm tempted to merge this if nobody else looks at it in the next few days.

@JakobOvrum
Copy link
Member

I did look over this minutes after it was filed, but I have no experience with DMD/Win64. That said, I understand what it does: that line of code is specific to Microsoft's C runtime, both 32-bit and 64-bit, but DMD only supported the Microsoft runtime for 64-bit before COFF was implemented for 32-bit as well very recently by @rainers himself.

So, in theory, LGTM :)

@quickfur
Copy link
Member

Should we just merge then? I'm not seeing the point of just letting this sit here and rot. It is just a 1-line change after all.

@JakobOvrum
Copy link
Member

Auto-merge toggled on

JakobOvrum added a commit that referenced this pull request Mar 24, 2015
Win32 COFF: fix std.process unittests
@JakobOvrum JakobOvrum merged commit f9c309f into dlang:master Mar 24, 2015
@JakobOvrum
Copy link
Member

@quickfur, agreed, thanks for paying attention :)

@yebblies
Copy link
Member

Does nobody care about Windows support?! It's been 4 days and not a single review...

@quickfur Most people don't care about win32 coff. We really need an autotester for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants