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: bump submodules #474

Merged
merged 11 commits into from
Dec 16, 2021
Merged

fix: bump submodules #474

merged 11 commits into from
Dec 16, 2021

Conversation

bruno-garcia
Copy link
Member

@bruno-garcia bruno-garcia commented Dec 15, 2021

NOTE: I had to push a branch to the .NET SDK since the addition of a .targets there broken the inheritance flow to this repo:

getsentry/sentry-dotnet#1397

I suggest we move on with this to unblock the next release. And iterate from there

@@ -152,6 +157,21 @@ jobs:
shell: pwsh
run: New-Item -ItemType SymbolicLink -Path "C:\${{ matrix.unity-version }}" -Target "C:\Program Files\Unity\Hub\Editor\${{ matrix.unity-version }}"

- name: Install Unity UPM Packages
Copy link
Member Author

Choose a reason for hiding this comment

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

These steps are all called by Build but to get their timing more clearly, I'm adding them here explicitly here.
They only run if their output hasn't being built yet so the time spent on it will move from the "Build Unity" step to here

@@ -168,7 +188,7 @@ jobs:
if: ${{ matrix.unity-version != '2022.1.0a12' }}
env:
UNITY_VERSION: ${{ matrix.unity-version }}
run: dotnet msbuild /t:UnityPlayModeTest /p:Configuration=Release /p:OutDir=other
run: dotnet msbuild /t:UnityPlayModeTest /p:Configuration=Release /p:OutDir=other test/Sentry.Unity.Tests
Copy link
Member Author

Choose a reason for hiding this comment

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

Added the project path explicitly so we're not running this target against the solution, and hence, against all projects in the solution. They are designed to spawn Unity once only

@@ -6,16 +6,19 @@
<AndroidBuildMethod>Builder.BuildAndroidIl2CPPPlayer</AndroidBuildMethod>
<AndroidBuildPath>$(PlayerBuildPath)Android/IL2CPP_Player.apk</AndroidBuildPath>
<!-- Assumes running `dotnet` from the root of the repo: -->
<UnitySampleProjectUnityVersion>$(SolutionDir)samples/unity-of-bugs/ProjectSettings/ProjectVersion.txt</UnitySampleProjectUnityVersion>
<RepoRoot>$([System.IO.Path]::GetDirectoryName($([MSBuild]::GetPathOfFileAbove('.gitignore', '$(MSBuildThisFileDirectory)'))))/</RepoRoot>
Copy link
Member Author

Choose a reason for hiding this comment

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

So we can build individual projects (dotnet build src/Sentry.Unity), we couldn't rely on SolutionDir because that didn't expand when not buiilding the .sln (which it's what we're doing when we do dotnet build on a directory with a .sln in it).

So changing things here to be relative to the root of the repo based on this new property

@@ -212,7 +214,8 @@ Related: https://forum.unity.com/threads/6572-debugger-agent-unable-to-listen-on
<!-- If Unity Library Project doesn't exist, create a Unity project. We use this project to restore packages needed to build
this solution without using the sample project which depends on the output of this build. -->
<Target Name="UnityCreatePackages"
Condition="!Exists('$(UnityPackageProject)') AND '$(MSBuildProjectName)' == 'Sentry.Unity'">
Condition="!Exists('$(UnityPackageProject)') AND '$(MSBuildProjectName)' == 'Sentry.Unity'"
AfterTargets="FindUnity">
Copy link
Member Author

Choose a reason for hiding this comment

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

$UnityPackageProject depends on FindUnity so this was missing

@@ -26,6 +22,7 @@
<Private>false</Private>
</Reference>
</ItemGroup>
<Error Condition="!Exists('$(UnityTestPath)/UnityEngine.TestRunner.dll')" Text="TestRunner not found. Expected: $(UnityTestPath)/UnityEngine.TestRunner.dll"></Error>
Copy link
Member Author

Choose a reason for hiding this comment

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

To help debug when UnityTestPath didnt' expand properly.

@@ -176,42 +176,6 @@ static SentryEvent CreateSentryEvent()
}
};
}

[Test]
public void SentrySdkCaptureEvent_WrongDsn_CorrectException()
Copy link
Member Author

Choose a reason for hiding this comment

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

This test isn't really doing anything other than testing DNS and our internal logging. Also, it started failing because of a change in the .NET SDK.

@bitsandfoxes bitsandfoxes merged commit d6883ad into main Dec 16, 2021
@bitsandfoxes bitsandfoxes deleted the fix/bump-submodules branch December 16, 2021 09:37
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