Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

feat: Returns better version data than default API #248

Merged
merged 3 commits into from
May 5, 2018

Conversation

bruno-garcia
Copy link
Member

@bruno-garcia bruno-garcia commented May 4, 2018

See getsentry/sentry#8314 for screenshots

Copy link
Contributor

@asbjornu asbjornu left a comment

Choose a reason for hiding this comment

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

LGTM!


<ItemGroup Condition="'$(TargetFramework)' == 'net35' or '$(TargetFramework)' == 'net40' or '$(TargetFramework)' == 'net45'">
Copy link
Contributor

Choose a reason for hiding this comment

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

The references are not required on .NET 3.5 or 4.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just moved it to their own blocks as it was tricky to follow what target gets what

return Type.GetType("Mono.Runtime", false)
?.GetMethod("GetDisplayName", BindingFlags.NonPublic | BindingFlags.Static)
?.Invoke(null, null)
is string monoVersion
Copy link
Contributor

Choose a reason for hiding this comment

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

The line breaking here is a bit funky. 😄 Perhaps breaking the expression up a bit would make it more readable as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, did reformat a bit to improve readability.
I do believe it might be about getting used to pattern matching syntax a bit.

@asbjornu
Copy link
Contributor

asbjornu commented May 4, 2018

The Travis build is failing, though?

* Method call is guarded by runtime check .NET so no need to test
directly which fails on Mono
@bruno-garcia
Copy link
Member Author

Travis failed because I ran the Registry code directly from test (without the IsNetFx guard) and that failed on Mono on Linux.
Since that's tested indirectly, I removed the test.

@bruno-garcia bruno-garcia merged commit d8e6938 into develop May 5, 2018
@bruno-garcia bruno-garcia deleted the feat/improve-runtime-info branch May 5, 2018 08:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants