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

removing crash dump processing logic from this solution... #44

Merged
merged 2 commits into from
Aug 6, 2021

Conversation

brianwp3000
Copy link
Contributor

...it's going to be in VmAgent sln. Also added CrashDumpState to SessionHostHeartbeatInfo

… to be in VmAgent sln. Added CrashDumpState to SessionHostHeartbeatInfo
@brianwp3000 brianwp3000 requested a review from IvanLH August 4, 2021 23:02
@@ -9,7 +9,7 @@
-->
<GeneratePackageOnBuild>True</GeneratePackageOnBuild>
<PackageId>PlayFab.MultiplayerServers.AgentInterfaces</PackageId>
<PackageVersion>5.1.5</PackageVersion>
<PackageVersion>5.2.0</PackageVersion>
Copy link
Contributor

Choose a reason for hiding this comment

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

why not 5.1.6? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is the definition of a minor version change--it's a backwards-compatible change to the public API (i.e. i added a field to SessionHostHeartbeatInfo)

@@ -3,7 +3,7 @@
<PropertyGroup>
<TargetFrameworks>netcoreapp3.1</TargetFrameworks>
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<PackageVersion>1.1.10</PackageVersion>
<PackageVersion>1.2.0</PackageVersion>
Copy link
Contributor

Choose a reason for hiding this comment

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

why not 1.1.11? :)

Copy link
Contributor Author

@brianwp3000 brianwp3000 Aug 4, 2021

Choose a reason for hiding this comment

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

This change should technically be a major version bump--i removed a method from a public-facing interface (ISessionHostManager), which is a breaking change. I opted for bumping the minor version instead though because it was a method that wasn't in use yet.
I'd prefer bumping the major version over bumping the patch version, though, because changing the patch version implies that there is no change to the public API. We should only be bumping the patch version if it's an implementation change that doesn't affect compilation.

Copy link
Contributor

Choose a reason for hiding this comment

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

fair enough, though our internal code is the only consumer of these APIs. LGTM!

@brianwp3000 brianwp3000 merged commit 887807d into main Aug 6, 2021
@brianwp3000 brianwp3000 deleted the bport/moveCrashDumpProcessing branch August 6, 2021 20:36
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