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

Issue 10830151: VS Add-in more properties and improvements (Closed)

Created:
8 years, 4 months ago by tysand
Modified:
8 years, 4 months ago
Reviewers:
noelallen1
CC:
native-client-reviews_googlegroups.com
Visibility:
Public.

Description

VS Add-in more properties and improvements Changed pepper libs to use Debug and Release versions. Added hidden property containing the add-in version. Property is set on the project when project loads. Added wait-for-debugger-children flag to PPAPI and no-sandbox for debugging. Added add-in version number to the .addin file. Fixed cygwin dos path warning. Minor code improvements. Made message in install script better and catch all expected errors to make them pretty. Fixed create_package including svn directories. Added web server port number as an option to change in property pages. BUG=136414 Committed: https://code.google.com/p/nativeclient-sdk/source/detail?r=1407

Patch Set 1 #

Patch Set 2 : #

Total comments: 10

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+609 lines, -171 lines) Patch
M visual_studio/NativeClientVSAddIn/InstallerResources/NaCl/ImportAfter/NaCl.override.props View 1 chunk +2 lines, -2 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/InstallerResources/NaCl/Microsoft.Cpp.NaCl.default.props View 1 chunk +4 lines, -3 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/InstallerResources/NaCl/Properties/nacl_general.xml View 1 3 chunks +30 lines, -14 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/InstallerResources/NativeClientVSAddIn.AddIn View 0 chunks +-1 lines, --1 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/InstallerResources/PPAPI_Patch/ImportAfter/PPAPI.override.props View 1 chunk +2 lines, -3 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/InstallerResources/PPAPI_Patch/Microsoft.Cpp.[platform].default.props.patch View 2 chunks +2 lines, -1 line 0 comments Download
M visual_studio/NativeClientVSAddIn/InstallerResources/PPAPI_Patch/PlatformToolsets/v100/microsoft.cpp.[platform].v100.props.patch View 1 chunk +1 line, -1 line 0 comments Download
M visual_studio/NativeClientVSAddIn/InstallerResources/PPAPI_Patch/Props/ppapi_general.xml.patch View 1 2 chunks +16 lines, -0 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/InstallerResources/install.py View 7 chunks +37 lines, -13 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaCl.Build.CPPTasks.csproj View 1 chunk +1 line, -0 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClCompile.cs View 1 chunk +1 line, -0 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLib.cs View 1 chunk +1 line, -0 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs View 1 chunk +1 line, -1 line 0 comments Download
M visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Connect.cs View 1 2 4 chunks +139 lines, -26 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/NativeClientVSAddIn.csproj View 2 chunks +5 lines, -5 lines 0 comments Download
A + visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/NativeClientVSAddIn - For Testing.AddIn View 0 chunks +-1 lines, --1 lines 0 comments Download
D visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/NativeClientVsAddIn_Testing.AddIn View 0 chunks +-1 lines, --1 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/PluginDebuggerHelper.cs View 7 chunks +63 lines, -60 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Strings.resx View 3 chunks +12 lines, -3 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Strings.Designer.cs View 4 chunks +30 lines, -3 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Utility.cs View 1 4 chunks +66 lines, -2 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/UnitTests/PluginDebuggerHelperTest.cs View 1 2 chunks +14 lines, -1 line 0 comments Download
M visual_studio/NativeClientVSAddIn/UnitTests/ProjectSettingsTest.cs View 1 8 chunks +80 lines, -31 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/UnitTests/TestUtilities.cs View 1 2 3 chunks +48 lines, -0 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/create_package.py View 4 chunks +57 lines, -5 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
tysand
8 years, 4 months ago (2012-08-02 23:35:57 UTC) #1
noelallen1
A few minor issues and some questions. Fix then LGTM. http://codereview.chromium.org/10830151/diff/27/visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Connect.cs File visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Connect.cs (right): http://codereview.chromium.org/10830151/diff/27/visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Connect.cs#newcode110 ...
8 years, 4 months ago (2012-08-04 01:18:34 UTC) #2
tysand
8 years, 4 months ago (2012-08-04 02:07:34 UTC) #3
http://codereview.chromium.org/10830151/diff/27/visual_studio/NativeClientVSA...
File visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Connect.cs (right):

http://codereview.chromium.org/10830151/diff/27/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Connect.cs:110: ///
Receives notification after any generic VS command has executed.
On 2012/08/04 01:18:34, noelallen1 wrote:
> Could you explain why you are doing this in the comment?  It's non-obvious.

Done.

http://codereview.chromium.org/10830151/diff/27/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Connect.cs:137: /// here.
On 2012/08/04 01:18:34, noelallen1 wrote:
> Same... Good to document why in case that reason goes away in the future.

Done.

http://codereview.chromium.org/10830151/diff/27/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Connect.cs:165: string
name = config.Platform.Name;
Embarrassing, this was debugging code. Removed it. 
On 2012/08/04 01:18:34, noelallen1 wrote:
> Is 'name' ever used?

http://codereview.chromium.org/10830151/diff/27/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Connect.cs:166: string aaa
= config.Name;
Debugging code. Oops.
On 2012/08/04 01:18:34, noelallen1 wrote:
> 'aaa' what is this variable for?

http://codereview.chromium.org/10830151/diff/27/visual_studio/NativeClientVSA...
File
visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/NativeClientVSAddIn.csproj
(right):

http://codereview.chromium.org/10830151/diff/27/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/NativeClientVSAddIn.csproj:140:
<Content Include="NativeClientVSAddIn - For Testing.AddIn">
This is a file name.  I changed this because it seems that Visual Studio loads
.AddIn files in alphabetical order and if you have both
NativeClientVsAddIn.AddIn (the file we ship) along side the testing version then
the not-testing version is loaded instead which causes issues for development
(namely you need to deploy every time you make a change to the add-in). Changing
this to " - For Testing.AddIn" makes VS load the testing version if both files
are available which makes development easier since the testing version points to
the undeployed build location of the add-in dll file (meaning you don't need to
deploy to see changes).
On 2012/08/04 01:18:34, noelallen1 wrote:
> So this is not a filename?

Powered by Google App Engine
This is Rietveld 408576698