Skip to content

Commit

Permalink
Update patch set 9
Browse files Browse the repository at this point in the history
Patch Set 9:

> Patch Set 9:
> 
> > Patch Set 9:
> > 
> > > AFAIK that all correct.
> > > also:
> > > - MS are not likely to change this logic till SP1 (if even)
> > > - Meanwhile `GYP` based projects (i.e. `node-gyp`, `node`, `libuv` that I know of) have been floating patches of their own...
> > > 
> > > Detection logic aside, we should land either this or 453201, since all the VS2017 issues are becoming a major PITA
> > 
> > Okay, I agree that we should land something as this has dragged on too long and we should have VS2017 support.
> > 
> > For now, since
> > 1) we have no reason to think that the registry-based approach doesn't work, and that that's what GN is using,
> > 2) and that I don't know who can actually review the PS/C# code for correctness or support it besides you,
> > 
> > I think the registry approach is the better way to go, and hopefully MS will change their course. If it turns out that the registry approach breaks or becomes otherwise problematic, I'm fine w/ taking this code for both GYP and GN until we get a better solution.
> > 
> > Does that work for you?
> 
> If you mean 453201, sure!

Ohh, P.S. the C#/PS code has landed in `node-gyp` nodejs/node-gyp#1130 and `Boost/build` boostorg/build#170
But NM

Patch-set: 9
Reviewer: Gerrit User 1188132 <1188132@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
  • Loading branch information
Gerrit User 1188132 authored and Gerrit Code Review committed Apr 7, 2017
1 parent 5b967d2 commit ea22cfc
Showing 0 changed files with 0 additions and 0 deletions.

0 comments on commit ea22cfc

Please sign in to comment.