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

Modified the F# Template so that GitVersionInformation is a static class and not a module anymore #2314

Merged

Conversation

odalet
Copy link
Contributor

@odalet odalet commented Jun 7, 2020

Description

  • Modified the F# template:
    • Added namespace global at the top of the file (it seems to required for types but not modules)
    • Changed the module into a type decorated with AbstractClass and Sealed and with no constructor so that it generates a static class.
    • Changed the per-property template: replaced the let expressions with static member definitions.

NB: I didn't add the extra attributes I described in #2311 to property getters. They seem a lot of boilerplate and not useful at all.

Related Issue

Fixes #2311

Motivation and Context

These changes have the F# compiler generate less code behind the scene, hence this is more code coverage-friendly.

How Has This Been Tested?

  • Fixed the unit tests
  • Ensured the resulting type could be quieried through reflection (beware; in this F# case we need look for properties, not fields) both with C# and F# code (see ReflectionTests and ReflectionTestsFS in https://github.com/odalet/GitVersionTests)
  • Double-checked the results in ILSpy:
// GitVersionInformation
using Microsoft.FSharp.Core;
using System;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;

[Serializable]
[AbstractClass]
[Sealed]
[CompilerGenerated]
[ExcludeFromCodeCoverage]
[CompilationMapping(/*Could not decode attribute arguments.*/)]
public static class GitVersionInformation
{
	public static string Major => "1";

	public static string Minor => "2";

	public static string Patch => "3";

        ...
}

No more weird $GitVersionInformation type:

image

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@asbjornu asbjornu merged commit 974ae22 into GitTools:master Jun 11, 2020
@asbjornu
Copy link
Member

Thank you for your contributions, @odalet! 🙏

@odalet
Copy link
Contributor Author

odalet commented Jun 11, 2020

Thanks for merging!

This PR, though, like #2313, suffered from the 'dotnet format' failing check too... I wanted to fix this soon, but it seems it automagically fixed itself :-)

@odalet odalet deleted the feature/exclude-fs-code-from-coverage branch June 12, 2020 15:44
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.

[Improvement] GitVersion generated F# code should not participate in code coverage
2 participants