Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(353)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 12248006: [VS Addin] Fix parallel building. (Closed)

Created:
7 years, 10 months ago by Sam Clegg
Modified:
7 years, 10 months ago
Reviewers:
noelallen1, binji
CC:
native-client-reviews_googlegroups.com
Visibility:
Public.

Description

[VS Addin] Fix parallel building. There was a bug in the calling code for compiler_wrapper. New unit test added to prevent regression. BUG= Committed: https://code.google.com/p/nativeclient-sdk/source/detail?r=1502

Patch Set 1 : #

Total comments: 5

Messages

Total messages: 6 (0 generated)
Sam Clegg
7 years, 10 months ago (2013-02-12 00:17:31 UTC) #1
noelallen1
lgtm
7 years, 10 months ago (2013-02-12 00:23:16 UTC) #2
binji
lgtm https://codereview.chromium.org/12248006/diff/9/visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs File visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs (left): https://codereview.chromium.org/12248006/diff/9/visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs#oldcode96 visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs:96: dte_.Solution.Open(SolutionName_); not needed? https://codereview.chromium.org/12248006/diff/9/visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs File visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs (right): https://codereview.chromium.org/12248006/diff/9/visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs#newcode1 ...
7 years, 10 months ago (2013-02-12 00:45:27 UTC) #3
Sam Clegg
https://codereview.chromium.org/12248006/diff/9/visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs File visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs (right): https://codereview.chromium.org/12248006/diff/9/visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs#newcode1 visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs:1: // Copyright (c) 2013 The Chromium Authors. All rights ...
7 years, 10 months ago (2013-02-12 01:02:15 UTC) #4
Sam Clegg
https://codereview.chromium.org/12248006/diff/9/visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs File visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs (left): https://codereview.chromium.org/12248006/diff/9/visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs#oldcode96 visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs:96: dte_.Solution.Open(SolutionName_); On 2013/02/12 00:45:27, binji wrote: > not needed? ...
7 years, 10 months ago (2013-02-12 01:03:40 UTC) #5
binji
7 years, 10 months ago (2013-02-12 01:19:05 UTC) #6 
lgtm

https://codereview.chromium.org/12248006/diff/9/visual_studio/NativeClientVSA...
File visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs (right):

https://codereview.chromium.org/12248006/diff/9/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs:1: // Copyright
(c) 2013 The Chromium Authors. All rights reserved.
On 2013/02/12 01:02:15, Sam Clegg wrote:
> On 2013/02/12 00:45:27, binji wrote:
> > apparently, this is not necessary.
> 
> But this is a "new" file.

Ah, fair enough.

Powered by Google App Engine
This is Rietveld 408576698