Fix description of assembly-informational-format

[mgnslndh]

The documentation referred to 'InformationalVersion' but using this explicitly in the configuration caused an error:

Unable to format AssemblyInformationalVersion.  Check your format string: 'InformationalVersion' is not a member of type 'GitVersion.SemanticVersionFormatValues' (Parameter 'propertyOrFieldName').

SemanticVersionFormatValues has property named DefaultInformationalVersion which works fine in the configuration.

[asbjornu]

Thanks for your contributions, @mgnslndh. Would it be possible to add a test for this?

[mgnslndh]

What kind of test are you looking for?

Also, the documentation for assembly-file-versioning-format links to the page listing variables but it seems it is not the same set of variables available when resolving configuration string interpolations (this is why I fixed the documentation in the first place). Maybe we should list the other set of variables on the configuration page?

[asbjornu]

@mgnslndh, I was thinking of something like the following, which tests that all existing variables are documented:

GitVersion/src/GitVersionCore.Tests/DocumentationTests.cs

Lines 50 to 63 in fe17357

	[Test]
	public void VariableDocumentationIsUpToDate()
	{
	var variableDocumentationFile = ReadDocumentationFile("input/docs/more-info/variables.md");
	var variables = VersionVariables.AvailableVariables.ToList();
	variables.ShouldNotBeEmpty();
	foreach (var variable in variables)
	{
	variableDocumentationFile.ShouldContain(variable,
	Environment.NewLine + variableDocumentationFile);
	}
	}

[mgnslndh]

Nice, did not know you had those kind of tests. I'll take a stab at it.

[mgnslndh]

There are two tests for documentation and they are correct. What I found was that when resolving the value of a variable in the configuration it had a name that was not the same as explained in the documentation. Variables used by configuration seems to be from SemanticVersionFormatValues.cs but the variables described in documentation are from VersionVariables.cs. The former is used to feed values into the latter. It is only the DefaultInformationalVersion on SemanticVersionFormatValues that is missing from VersionVariables which instead has the InformationalVersion

Not sure how to proceed.

Should we have a separate documenation page describing which variables can be resolved by configuration settings and make a test that checks that all variables in SemanticVersionFormatValues are listed?

[asbjornu]

Hm, this is an unfortunate deviation that I think we should resolve. Optimally, SemanticVersionFormatValues shouldn't need to exist. They should at least only expand on what already exists in VersionVariables, not duplicate or rename properties. What we could do is start logging a warning if DefaultInformationalVersion is used and add support for InformationalVersin (and any other potentially missing VersionVariables). This should all be documented as well.

Would you be up for submitting a PR tackling this, @mgnslndh?

[mgnslndh]

The thing is the dependency is the other way around. VersionVariables is only instanciated at one place in the code base and then the values comes from SemanticVersionFormatValues. See https://github.com/GitTools/GitVersion/blob/master/src/GitVersionCore/OutputVariables/VariableProvider.cs#L57-L69

It looks like VersionVariables is just a simple container for the variables but SemanticVersionFormatValues contains logic (although not that much) that determines the value of some of the variables according to the configuration. However, it is the VariableProvider that knows that certain variables need to be "resolved" because the user might have provided custom values in the configuration.

So basically the new InformationalVersion works just like the current DefaultInformationalVersion and we change DefaultInformationalVersion to also omit a warning when it is used and remove it on next breaking release? I could submit a PR for this.

This PR however, seems like it should be closed. If we rename the variable the documentation would be correct in its current state.

Also, just for reference. It seems the property has always been named like this. See 684aa07#diff-2217934fd9282cd4fd7c5ba8e030b81eR111-R114

[asbjornu]

So basically the new InformationalVersion works just like the current DefaultInformationalVersion and we change DefaultInformationalVersion to also omit a warning when it is used and remove it on next breaking release?

Yes.

I could submit a PR for this.

That would be great!

This PR however, seems like it should be closed. If we rename the variable the documentation would be correct in its current state.

Indeed.

[asbjornu]

I suppose this can be closed now that #2051 is merged?

[mgnslndh]

Yes, fine by me!