search

Azure / azure-sdk-for-cpp

This repository is for active development of the Azure SDK for C++. For consumers of the SDK we recommend visiting our versioned developer docs at https://azure.github.io/azure-sdk-for-cpp.
MIT License
178 stars 128 forks source link

Avoid double negative and rename the CurlTransportSSLOptions.NoRevoke to be something like CheckCertificateRevocationList #1102

Closed ahsonkhan closed 3 years ago

ahsonkhan commented 3 years ago

Since we expose this to our customers as a public API, consider renaming it to avoid the double negative and direct rather than trying to follow libcurl's naming.

https://github.com/Azure/azure-sdk-for-cpp/blob/6ff9b7b60558717db1c0d012145cb9799aa0d2b1/sdk/core/azure-core/inc/azure/core/http/curl/curl.hpp#L26-L30

Names that are more self-documenting and declarative (use the Disable or Enable prefix depending on where we settle for the default value - https://github.com/Azure/azure-sdk-for-cpp/issues/1101):

Frequently, we have observed confusion on what NO_REVOKE means especially when it is false, since it is exposed as a boolean.

FWIW, WinHTTP uses WINHTTP_ENABLE_SSL_REVOCATION and .NET's HttpClientHandler uses CheckCertificateRevocationList

See https://github.com/Azure/azure-sdk-for-cpp/pull/885#discussion_r518325905

cc @RickWinter, @JeffreyRichter, @vhvb1989

vhvb1989 commented 3 years ago

I prefer to use the same name as libcurl since people could be already familiar with it. So, asking @RickWinter to confirm if we want to do this change or not.

When it comes to settings that we are going to apply to libcurl handle, I personally prefer to maintain the libcurl's original property name.

However, it's fine if Rick wants to change it for the SDK.

RickWinter commented 3 years ago

For each value we add, we need to think about the future of the switch. If the switch is generic across transports (or could be) we need to choose a name which makes sense and is consistent for all transports. The switch name also needs to take into account the other sdk languages and the names they use. We want consistency where it makes sense.