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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 11145025: Only store major version of AddIn in VS project file (Closed)

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

Description

Only store major version of AddIn in VS project file. This avoids vcproj file churn. Also, get the version number directly from the assembly rather than parsing the .AddIn file. Also, fix for default debugger args. Without this change the PPAPI default debugger args don't work since TargetPath ends up being empty. The side effect would be that the PPAPI project would fail to run if the user deleted their .user file. BUG= Committed: https://code.google.com/p/nativeclient-sdk/source/detail?r=1450

Patch Set 1 #

Patch Set 2 : #

Total comments: 6

Patch Set 3 : #

Messages

Total messages: 3 (0 generated)
Sam Clegg
8 years, 2 months ago (2012-10-15 19:53:53 UTC) #1
binji
lgtm http://codereview.chromium.org/11145025/diff/2001/visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/AssemblyInfo.cs File visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/AssemblyInfo.cs (right): http://codereview.chromium.org/11145025/diff/2001/visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/AssemblyInfo.cs#newcode30 visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/AssemblyInfo.cs:30: //[assembly: AssemblyFileVersion("1.0.*")] not necessary? If so, remove. http://codereview.chromium.org/11145025/diff/2001/visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Connect.cs ...
8 years, 2 months ago (2012-10-15 20:00:58 UTC) #2
Sam Clegg
8 years, 2 months ago (2012-10-15 20:07:51 UTC) #3 
http://codereview.chromium.org/11145025/diff/2001/visual_studio/NativeClientV...
File visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/AssemblyInfo.cs
(right):

http://codereview.chromium.org/11145025/diff/2001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/AssemblyInfo.cs:30:
//[assembly: AssemblyFileVersion("1.0.*")]
On 2012/10/15 20:00:59, binji wrote:
> not necessary? If so, remove.

Done.

http://codereview.chromium.org/11145025/diff/2001/visual_studio/NativeClientV...
File visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Connect.cs (left):

http://codereview.chromium.org/11145025/diff/2001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Connect.cs:230: 
On 2012/10/15 20:00:59, binji wrote:
> Why did you remove this?

This was here because the only way to get the args to work was to have them
stored in the .user file.

The default args didn't work because $(TargetPath) was not
defined.  I fixed this issue so it is no longer necessary
to copy this property.

http://codereview.chromium.org/11145025/diff/2001/visual_studio/NativeClientV...
File visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Connect.cs (right):

http://codereview.chromium.org/11145025/diff/2001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Connect.cs:265: /// Get
the major version of the AddIn
On 2012/10/15 20:00:59, binji wrote:
> nit: period at end of sentence.

Done.

Powered by Google App Engine
This is Rietveld 408576698