Update documentation from JS comments #1381
Conversation
momuno
added
Docs
Client
| } | ||
| ``` | ||
|
|
||
| ### IoT Hub Client with MQTT Stack |
There was a problem hiding this comment.
I'm going to push back on this one. Why is this being removed? I think John's original comment is that the readme is a little long but that was attempted to be solved with the table of contents.
There was a problem hiding this comment.
For two reasons, 1, John's comment about the readme being too long, and 2, because this code is accomplished in the samples. I see it attempting to be MQTT stack agnostic here, but I actually found that to be more confusing since the mqtt function calls appeared to be hypothetical? I would prefer to direct the user to the samples at this point.
There was a problem hiding this comment.
I have added a coding_patterns.md file that now houses this.
What comments are you referring to? Are those from another PR (and if so can you share a link for context)? JS - JavaScript? Edit: Oh, JS - John Spaith? |
Yes, John Spaith lol. I was trying not to yell his name across the PR page, so attempted a more discreet approach. |
|
Creating a new PR to get tests to pass (they are stuck in fail but should be passing now) |
|
Checks failing again. Closing this and re-opening other PR #1415 so checks pass. |
This reverts commit 5a08664.
…or-c into momuno/doc-updates-js
…re-sdk-for-c into momuno/doc-updates-js
| @@ -31,6 +31,7 @@ steps: | |||
| | "$(VcpkgCommit)" | |||
| | $(Agent.Os) | |||
| | $(${{ parameters.DependenciesVariableName }}) | |||
| | $(VCPKG_DEFAULT_TRIPLET) | |||
There was a problem hiding this comment.
@weshaggard, @danieljurek - We are adding the triplet name to the restore key of the cache to make it more unique and force a cache miss.
This was causing issues because we temporarily changed the triplet value to solve a different issue (and reverted it back), but since it is still within the 7 day window, the cache had the incorrect packages stored corresponding to the wrong triplet (x86-windows-static-md instead of x86-windows-static).
For context:
- The temporary change to the triplet was made here - Fix libcmt/libcmtd.lib link warning #1338
- Which we reverted back to what it originally was here - Fix Windows to use /MT when building static libraries. #1394
Clearing a cache is currently not supported. However you can add a string literal (e.g. version2) to your existing cache key to change the key in a way that avoids any hits on existing caches.
See this Windows leg, which now has a cache miss since we now include the triplet name as part of the key:
https://dev.azure.com/azure-sdk/public/_build/results?buildId=566055&view=logs&j=1822ef0b-3ac4-586f-233b-6fdf166a140f&t=47e4b218-8f82-5039-fb29-8b3a261ff980
Resolved to: Validate WindowsX86_Release_MapFiles_UnitTests|"tags/2020.06"|Windows_NT|cmocka|x86-windows-static
ApplicationInsightsTelemetrySender will correlate events with X-TFS-Session b3cd6573-8ace-44b3-8bb2-82ce64481824
Getting a pipeline cache artifact with one of the following fingerprints:
Fingerprint:Validate WindowsX86_Release_MapFiles_UnitTests|"tags/2020.06"|Windows_NT|cmocka|x86-windows-static
There is a cache miss.
There was a problem hiding this comment.
FYI, @vhvb1989, @antkmsft, @RickWinter, if this issue comes up again.
Once we have a new release of Vcpkg, we can leverage their own binary caching feature which might be less error-prone and flexible, moving forward: microsoft/vcpkg#11204
There was a problem hiding this comment.
@ahsonkhan please comment the vcpkg.yml file with these details. Years from now it will be helpful to know why that triplet is there and I doubt anyone will remember this issue
There was a problem hiding this comment.
@RickWinter 2 months later, I remembered ahahaha.. Happened again here: Azure/azure-sdk-for-cpp#1042
There was a problem hiding this comment.
please comment the vcpkg.yml file with these details.
No description provided.