Victorhu/fileversion #1529
Victorhu/fileversion #1529
Conversation
ghost
added
the
|
|
||
| // If the assembly version is empty then set the version | ||
| if (compilation.Assembly.Identity.Version == emptyVersion) | ||
| var addAttributeIfNotExists = new Action<string, string>((attributeName, attributeValue) => |
There was a problem hiding this comment.
Why not make this a regular method?
There was a problem hiding this comment.
Less arguments :)
|
@pranavkm updated the PR. please take a look |
| } | ||
| catch(FormatException ex) | ||
| { | ||
| throw new FormatException("The assembly file version is invalid: " + fileVersion, ex); |
There was a problem hiding this comment.
Nah. Just use Version.Parse and let that throw.
There was a problem hiding this comment.
Nah. The error message is really bad. It just says that the string has an invalid format but doesn't tell you what the value is.
There was a problem hiding this comment.
Then at least fix the formatting in the file 😄
|
Updated |
| { | ||
| try | ||
| { | ||
| project.AssemblyFileVersion = new Version(fileVersion); |
There was a problem hiding this comment.
Maybe Version.TryParse instead of try-catching?
|
|
once somebody from DNX signs off.
| } | ||
| catch (FormatException ex) | ||
| { | ||
| throw new FormatException("The assembly file version is invalid: " + fileVersion, ex); |
There was a problem hiding this comment.
Consider using String.Format instead of concat.
There was a problem hiding this comment.
Doesn't the exception message have to be localizable?
There was a problem hiding this comment.
No strings in the runtime are localizable. I think there is an issue about that somewhere...
|
/cc: @joeloff
|
|
Updated the PR with one change: the prefix for the version is picked up from the |
victorhurdugaci
force-pushed
the
victorhu/fileversion
branch
from
718117d to
ed5b609
Compare
victorhurdugaci
force-pushed
the
victorhu/fileversion
branch
from
ed5b609 to
a50b85c
Compare
Inject the file version attribute in the generated assembly, only if the attribute is not already present.
Goes together with aspnet/Universe#192 and fixes #1504
Please review: @davidfowl, @anurse, @pranavkm, @ChengTian
Please note this is for beta4/release branch