jhump commented on code in PR #3266: URL: https://github.com/apache/avro/pull/3266#discussion_r1932296881
########## lang/c++/impl/CustomAttributes.cc: ########## @@ -16,26 +16,63 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include "CustomAttributes.hh" -#include "Exception.hh" + +#if defined(__clang__) +// Even though CustomAttributes::ValueMode::STRING is deprecated, we still have to +// handle/implement it. +#pragma clang diagnostic ignored "-Wdeprecated-declarations" Review Comment: That doesn't work here. It's the implementation in the body of this file that references the deprecated elements. Doing this only for the `#include` does nothing. If I did use `push|pop`, we'd have to do it around all three functions that have offending usages: the constructor, `addAttribute`, and `printJson`. That starts to get very verbose, and it's unclear that it's helpful: if the implementation added another method that needed to handle `ValueMode::STRING`, we'd have to wrap yet another place. So it feels appropriate to have it apply to the whole file. I can move it below to make it clear that it applies to the implementation body, not an include directive. I **can**, however, use `push|pop` in `uinttest.cpp`, since it's only one function there that tests (and thus uses) the deprecated elements. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@avro.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
