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 GitVersionInformationAttribute to make reflection easier #526

Closed
wants to merge 6 commits into from
Closed

Add GitVersionInformationAttribute to make reflection easier #526

wants to merge 6 commits into from

Conversation

asbjornu
Copy link
Member

Right now, it's a bit hard to do reflection on an assembly to dig out all of the juicy GitVersion data, since the data is embedded within the static class GitVersionInformation. To get to this class, you'd have to do Assembly.GetTypes(), which is prone to throwing exceptions. It would be more reflection-friendly to have the information inside an [assembly: Attribute].

So I've created a GitVersionInformationAttribute that should remedy this. I've not removed any of the previously generated data, but I've taken the liberty of removing extraneous whitespace in the generated .cs file. I've also expanded on the AssemblyInfoBuilderTests.VerifyCreatedCode() test to verify the contents of the generated assembly and assert whether or not the GitVersionInformationAttribute actually contains the information we want to or not.

{
var emitResult = compilation.Emit(stream);

Assert.IsTrue(emitResult.Success, string.Join(Environment.NewLine, emitResult.Diagnostics.Select(x => x.Descriptor)));
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI shouldly would give much better error messages:

Current: false should be true (some descriptor)
Shouldly: emitResult.Success should be true. Additional Context: some descriptor

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, it's not my assert. I thought I'd just leave as much code I didn't write untouched to focus the PR on one thing so it would be easier to review. 😄

@JakeGinnivan
Copy link
Contributor

#525 puts the different classes into namespaces which prevents conflicts when using InternalsVisibleTo. I am going to merge that PR first, if you could rebase this and put this new attribute under the assemblies namespace

using GitVersion;

public class AssemblyInfoBuilder
{
public string GetAssemblyInfoText(VersionVariables vars)
{
var v = vars.ToArray();
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, .ToList() is actually more efficient because an IEnumerable length is not fixed the array buffer has to be expanded multiple times, lists have that functionality built in. Not that it really matters in most code but just a FYI :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes. I just like arrays better due to the built in syntactic sugar. Not that I used that here, but habits die hard. :)

@JakeGinnivan
Copy link
Contributor

Nice addition, have rebased and pushed to #531 so it can be merged

@asbjornu
Copy link
Member Author

@JakeGinnivan Awesome! 😄

@asbjornu asbjornu deleted the feature/versioninfo-attribute branch July 22, 2015 11:30
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.

2 participants