Json library update and Xcode15 #5681
Conversation
The code within the `mocked_transport_adapter_test.hpp` file, specifically within the `Azure::Security::KeyVault::Keys` namespace, has been updated. The `#if defined(_MSC_VER)` preprocessor directive along with its associated `#pragma warning(push)` and `#pragma warning(disable : 4996)` directives have been removed. The corresponding `#pragma warning(pop)` directive has also been removed. The lines of code that were between these preprocessor directives remain unchanged. These changes remove specific compiler warning suppressions for MSVC compilers, but do not alter the functional behavior of the code.
...ssaging-eventhubs/test/perf/inc/azure/messaging/eventhubs/test/eventhubs_batch_perf_test.hpp
Show resolved
Hide resolved
|
/azp run cpp - core |
|
Azure Pipelines successfully started running 1 pipeline(s). |
while i would agree with you there are api changes in the internal impl of the tests , the old ones don't compile properly , thus i had to update. As mentioned in the description i ran all the CI pipelines that IMHO cover most if not all of the public interface , thus the functionality out of it that we use is covered , if we want to run some coverage tools to determine dead code in the json that we don't use that i would like to do to remove unneeded functionality, only that future updates would be harder to perform as we would need to determine what we removed and where it is and get rid of them again. We can talk about it more monday, i did not bring this up thursday as that meeting was too clogged with go lang issues :) |
So be it. I was hoping to avoid doing even more work to update the tests, but if the existing tests break, so be it. |
|
Reminder, in case this got lost -
I can now see |
done, apologies , i somehow missed that |
|
/azp run cpp - core |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Just to understand, how come we remove
|
There was a problem hiding this comment.
I eye-balled the diff between nlohmann json and our vendored copy, and it looks reasonable.
Seems like most of the changes are covered by:
- Adding the
_azure_prefix to macros - Modifying the namespaces
- Clang-format formatting
Thanks for updating the library, @gearama!
|
@ahsonkhan, similar to #5681 (comment). #ifndef X_HPP_INCLUDED
#define X_HPP_INCLUDED
// header file contents
#endif it is called "include guard". It is basically the same as #pragma once
// header file contents
It is in our guidelines for C++ to use This is maybe not so often the case with C compilers, BTW, especially maybe for some exotic embedded platforms, which is why in Embedded C repo, we were using include guards and not pragmas. |
|
@gearama, could you please drop all the occurrences of /*!
@brief namespace for Niels Lohmann
@see
https://github.com/nlohmann
@since version 1.0.0
*/? The reason is that I used doxygen to generate docs, and what it does is that it ends up attaching |
|
/azp run cpp - core |
|
Azure Pipelines successfully started running 1 pipeline(s). |
closes #5670
Updated json library
Updated macos tests to ver 14
Readmes have been touched with add/remove spaces at the end to trigger all Ci pipelines in order to run all tests to verify the json library behaves as expected
Ran all the CI pipelines , since i have to revert the readme files i paste the result here
Pull Request Checklist
Please leverage this checklist as a reminder to address commonly occurring feedback when submitting a pull request to make sure your PR can be reviewed quickly:
See the detailed list in the contributing guide.