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

Fix: Add product name to release as default #202

Merged
merged 6 commits into from
May 27, 2021

Conversation

bitsandfoxes
Copy link
Contributor

Release should be productName@version if it's not overwritten by the user.

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

nice!

src/Sentry.Unity/SentryUnity.cs Outdated Show resolved Hide resolved
@bitsandfoxes bitsandfoxes merged commit 07a3543 into main May 27, 2021
@bitsandfoxes bitsandfoxes deleted the fix/release-productname branch May 27, 2021 19:01
@@ -32,7 +32,8 @@ public static void Init(SentryUnityOptions unitySentryOptions)
// Uses the game `version` as Release unless the user defined one via the Options
if (unitySentryOptions.Release == null)
{
unitySentryOptions.Release = Application.productName + "@" + Application.version;
unitySentryOptions.Release = $"{Application.productName}@{Application.version}";
Copy link
Member

Choose a reason for hiding this comment

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

Just realized something: Can productName be null or an empty string? We might want to check for that before appending @

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can not be null or empty. But it could be just a whitespace. To what should we default in that case? Just the version number without the @?

Copy link
Member

Choose a reason for hiding this comment

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

yeah, we only append the @ if there's a string to its left. But if users are 'forced' to set something it's more of an edge case and not a huge deal.

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