Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support and tests for AssemblyInformationalVersion configurability #660

Merged
merged 3 commits into from
Oct 17, 2015

Conversation

MarkZuber
Copy link
Contributor

No description provided.

@JakeGinnivan
Copy link
Contributor

Thanks for rebasing this @MarkZuber

Just wondering if it would be better to instead call it assembly-informational-override then specify a GitVersion variable. For instance assembly-informational-override: NuGetVersion, but you can also go assembly-informational-override: FullSemVer

Another option is to have assembly-informational-format: {NuGetVersion}, sha: {Sha}, then you can format however you want. I would suggest maybe using https://www.nuget.org/packages/netfx-System.StringFormatWith/ if you wanted to go down this road, if that doesn't do the job then have a look at http://haacked.com/archive/2009/01/14/named-formats-redux.aspx/ for some code to use.

@asbjornu
Copy link
Member

asbjornu commented Oct 5, 2015

I'd prefer a string interpolation based solution with named variables. :)

@MarkZuber
Copy link
Contributor Author

OK. I've looked at StringFormatWith and it works great except the code
pulled in from the nuget package is older .net and has a few conflicts as
is. It's one file, so i pulled that code in (with appropriate
attributions) into the gitversioncore project.

all tests currently pass and i'm writing some additional tests now to cover
a few additional formatting cases. hope to submit an updated pull request
later today.

thanks for the feedback and input.

-mark

On Mon, Oct 5, 2015 at 12:19 AM, Asbjørn Ulsberg [email protected]
wrote:

I'd prefer a String.Format-based solution with named variables. :)


Reply to this email directly or view it on GitHub
#660 (comment).

Updated the code to provide assembly-informational-format in the yaml
config to customize how the assembly informational data will be
formatted.
@MarkZuber
Copy link
Contributor Author

Quick ping. Does everything look acceptable? Would love to get this into mainline and get a real drop. Please let me know if there are any other changes you'd like me to make. thanks!

@pascalberger
Copy link
Member

Does this also fix #368?

@@ -0,0 +1,116 @@
namespace GitVersion
{
public class SemanticVersionFormatValues
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we use the VersionVariables class for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need this class in order to generate the VersionVariables class since the types we're using int he formatting need to come from a constructed object. And if we exposed the VersionVariables object as what's valid for formatting, then someone could do {InformationalVersion} and get themselves into an infinite loop. So this gives us type safety of what's allowed to be formatted as well as getting the formatting done to construct the VersionVariables type. I agree it's redundant on most fields (since VersionVariables now just becomes a passthrough/holder for most of the values calculated in the SemanticVersionFormatValues class. Open to suggestions to minimize here if you have them.

@JakeGinnivan
Copy link
Contributor

Looking really good @MarkZuber, just a few comments on there. Thanks!

@MarkZuber
Copy link
Contributor Author

Ping... I think i've answered/fixed all of the issues requested on the feedback. Would love to get this merged so we can pick up an official drop to use on our build servers. thanks!

JakeGinnivan added a commit that referenced this pull request Oct 17, 2015
Add support and tests for AssemblyInformationalVersion configurability
@JakeGinnivan JakeGinnivan merged commit 73beefa into GitTools:master Oct 17, 2015
@MarkZuber MarkZuber deleted the feature/assemblyInfoUpdates branch October 20, 2015 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants