Issue #31: ETW APIs are unimplemented#213
Conversation
|
Hi @CBaud, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
TTYL, MSBOT; |
MattHyman
assigned
MattHyman and
david-c-kline and unassigned
MattHyman and
david-c-kline
WilliamsJason
left a comment
WilliamsJason
left a comment
There was a problem hiding this comment.
Looks good to me! Would be good to get at least one other person to look at these then we should merge them in. If nobody has time in the next day or two, I'll just merge them in anyways.
Looks like the websocket changes would be considered breaking changes for anyone consuming the DLL if they update to a newer version, is that correct? If so we might want to update the version number as well and make sure we document the change.
| /// <param name="etwEvents">The <see cref="EtwEvents"/> to validate.</param> | ||
| private static void ValidateEtwEvents(EtwEvents etwEvents) | ||
| { | ||
| Assert.IsNotNull(etwEvents); |
There was a problem hiding this comment.
Don't need to check event list elements?
|
Great work! Nice to see this support. |
| { | ||
| TestHelpers.MockHttpResponder.AddMockResponse(DevicePortal.EtwProvidersApi, HttpMethods.Get); | ||
|
|
||
| Task<EtwProviders> getEtwProvidersTask = TestHelpers.Portal.GetEtwProvidersAsync(); |
There was a problem hiding this comment.
I believe that the VS test infrastructure supports making methods 'public async Task' so you can 'await' on the task to make things a little cleaner and more representative; i.e.
EtwProviders etwProviders = await TestHelpers.Portal.GetEtwProvidersAsync();
|
|
||
| /// <summary> | ||
| /// The connection to the websocket. | ||
| /// </summary> |
There was a problem hiding this comment.
You're adding an IDisposable member - shouldn't you add an IDisposable implementation to this class? That being said, there was already an IDisposable member, so perhaps it's unnecessary?
|
This is fantastic! Thanks for implementing this, Chris! |
7 participants
No description provided.