Skip to content

Comments

Add a per-SteamClient unique identifier message to DebugLog messages (2)#777

Merged
yaakov-h merged 5 commits intoSteamRE:masterfrom
yaakov-h:experiment/debuglog-id
Nov 4, 2019
Merged

Add a per-SteamClient unique identifier message to DebugLog messages (2)#777
yaakov-h merged 5 commits intoSteamRE:masterfrom
yaakov-h:experiment/debuglog-id

Conversation

@yaakov-h
Copy link
Member

@yaakov-h yaakov-h commented Nov 3, 2019

A different approach to solving #503.

Thoughts?

This version should not trigger breaking changes in consumers.

cc/ @JustArchi @xPaw

@yaakov-h yaakov-h added this to the 2.3.0 milestone Nov 3, 2019
/// <summary>
/// A unique identifier for this client instance.
/// </summary>
public string ID { get; }
Copy link
Member

Choose a reason for hiding this comment

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

Allow setter?

Copy link
Member Author

Choose a reason for hiding this comment

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

Should be immutable for the object’s lifetime, so perhaps a constructor parameter?

Copy link
Member

Choose a reason for hiding this comment

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

That works.

/// <summary>
/// A unique identifier for this client instance.
/// </summary>
public string ID { get; }
Copy link
Contributor

Choose a reason for hiding this comment

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

All fine but a setter would indeed be nice, see my reasoning in #503 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

See above.

@codecov
Copy link

codecov bot commented Nov 4, 2019

Codecov Report

Merging #777 into master will increase coverage by 0.02%.
The diff coverage is 12.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #777      +/-   ##
==========================================
+ Coverage   22.74%   22.77%   +0.02%     
==========================================
  Files          94       95       +1     
  Lines        9306     9329      +23     
  Branches      770      772       +2     
==========================================
+ Hits         2117     2125       +8     
- Misses       7055     7070      +15     
  Partials      134      134
Impacted Files Coverage Δ
...2/Networking/Steam3/NetFilterEncryptionWithHMAC.cs 0% <0%> (ø) ⬆️
SteamKit2/SteamKit2/Util/DebugNetworkListener.cs 0% <0%> (ø) ⬆️
...mKit2/SteamKit2/Networking/Steam3/TcpConnection.cs 0% <0%> (ø) ⬆️
...SteamKit2/Networking/Steam3/WebSocketConnection.cs 0% <0%> (ø) ⬆️
...mKit2/SteamKit2/Networking/Steam3/UdpConnection.cs 0% <0%> (ø) ⬆️
...t2/SteamKit2/Networking/Steam3/WebSocketContext.cs 0% <0%> (ø) ⬆️
...SteamKit2/Networking/Steam3/NetFilterEncryption.cs 0% <0%> (ø) ⬆️
SteamKit2/SteamKit2/Util/ILogContext.cs 100% <100%> (ø)
...eamKit2/SteamKit2/Steam/SteamClient/SteamClient.cs 47.08% <77.77%> (+1.58%) ⬆️
SteamKit2/SteamKit2/Util/ZipUtil.cs 0% <0%> (ø) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 627226a...b3cbc64. Read the comment docs.

@yaakov-h yaakov-h marked this pull request as ready for review November 4, 2019 10:40
@yaakov-h yaakov-h merged commit 58562fc into SteamRE:master Nov 4, 2019
@yaakov-h yaakov-h deleted the experiment/debuglog-id branch November 4, 2019 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants