Minor code cleanups 2 #6171
Minor code cleanups 2 #6171
Conversation
antkmsft
requested review from
gkostal,
anilba06,
ahmadmsft,
LarryOsterman,
RickWinter,
schaabs,
ahsonkhan,
gearama and
a team
as code owners
antkmsft
requested review from
maorleger,
billwert,
KarishmaGhiya,
mpodwysocki and
minhanh-phan
|
API change check API changes are not detected in this pull request. |
|
@LarryOsterman, here is what it sees as API changes: https://apiview.dev/Assemblies/Review/30c9d4a8b05d43dbb57900fb4f9329f7/b7b4144a10a944ffb5ca126c01165d02?diffRevisionId=06e369ba41614f1eab19d30c565c7484&doc=False&diffOnly=True It must be the |
| @@ -327,7 +327,7 @@ namespace Azure { namespace Core { namespace Http { | |||
| * @brief A value indicating whether the returned raw response for this request will be buffered | |||
| * within a memory buffer or if it will be returned as a body stream instead. | |||
| */ | |||
| bool ShouldBufferResponse() { return this->m_shouldBufferResponse; } | |||
| bool ShouldBufferResponse() const { return this->m_shouldBufferResponse; } | |||
There was a problem hiding this comment.
To confirm, removing const from a method is considered a breaking change, correct?
But adding is fine. Because a const method can be called on either a const or non-const instance, whereas a non-const method cannot be called by a const instance.
In general, I wonder how pessimistic we should be with adding const, to avoid a breaking change, if the implementation were required to change.
| @@ -8,10 +8,10 @@ | |||
|
|
|||
| namespace { | |||
|
|
|||
| static char const Base64EncodeArray[65] | |||
There was a problem hiding this comment.
What's the rationale here to remove static?
There was a problem hiding this comment.
Not 100% sure if this is the logic from the tools that Anton is running, but declaring the type as "static" removes the symbol from the symbol table, which makes it impossible to address from outside the module.
But putting the type in an anonymous namespace has the identical effect, so it mostly just makes it harder to debug things.
No description provided.