From 55fe93a2c33fdeb093e389911109f424858b222f Mon Sep 17 00:00:00 2001
From: ClausKlein <claus.klein@arcormail.de>
Date: Fri, 12 Feb 2021 16:36:19 +0100
Subject: [PATCH] respect most review comments

no default cxx language settings
install fmt
link against fmt::fmt
export dependency too if not needed
...
---
 CMakeLists.txt             | 21 +++++++--------------
 include/greeter/greeter.h  |  3 ---
 source/greeter.cpp         | 15 +++++----------
 standalone/source/main.cpp | 26 +++++++++++++++++---------
 test/source/greeter.cpp    |  5 -----
 5 files changed, 29 insertions(+), 41 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 2e0e3ae..eb2d3e4 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -18,14 +18,6 @@ if(PROJECT_SOURCE_DIR STREQUAL PROJECT_BINARY_DIR)
   )
 endif()
 
-# ---- Project settings ----
-
-if(NOT DEFINED CMAKE_CXX_STANDARD)
-  set(CMAKE_CXX_STANDARD 17)
-  set(CMAKE_CXX_STANDARD_REQUIRED ON)
-  set(CMAKE_CXX_EXTENSIONS NO)
-endif()
-
 # ---- Add dependencies via CPM ----
 # see https://github.com/TheLartians/CPM.cmake for more info
 
@@ -61,17 +53,17 @@ file(GLOB_RECURSE sources CONFIGURE_DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/source/
 # target! EITHER: add_library(Greeter INTERFACE) OR:
 add_library(Greeter ${headers} ${sources})
 
-set_target_properties(Greeter PROPERTIES CXX_STANDARD ${CMAKE_CXX_STANDARD})
-# OR target_compile_features(Greeter PUBLIC cxx_std_17)
+# EITHER: set_target_properties(Greeter PROPERTIES CXX_STANDARD ${CMAKE_CXX_STANDARD}) # OR
+target_compile_features(Greeter PUBLIC cxx_std_17)
 
 # being a cross-platform target, we enforce standards conformance on MSVC
 if(MSVC)
   target_compile_options(Greeter PUBLIC /permissive)
 endif()
 
-# Link dependencies (if required)
-target_link_libraries(Greeter PRIVATE $<BUILD_INTERFACE:fmt::fmt-header-only>)
-# XXX target_link_libraries(Greeter PUBLIC fmt::fmt)
+# Link dependencies EITHER: target_link_libraries(Greeter PRIVATE
+# $<BUILD_INTERFACE:fmt::fmt-header-only>) # OR:
+target_link_libraries(Greeter PUBLIC fmt::fmt)
 
 target_include_directories(
   Greeter PUBLIC $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include>
@@ -93,6 +85,7 @@ packageProject(
   INCLUDE_DIR ${PROJECT_SOURCE_DIR}/include
   INCLUDE_DESTINATION include/${PROJECT_NAME}-${PROJECT_VERSION}
   VERSION_HEADER "${VERSION_HEADER_LOCATION}"
-  # TODO COMPATIBILITY SameMajorVersion
+  COMPATIBILITY ExactVersion
+  # TBD COMPATIBILITY SameMajorVersion
   DEPENDENCIES "fmt 7.1.3"
 )
diff --git a/include/greeter/greeter.h b/include/greeter/greeter.h
index 9d367d7..77dfe3b 100644
--- a/include/greeter/greeter.h
+++ b/include/greeter/greeter.h
@@ -26,9 +26,6 @@ namespace greeter {
      * @return a string containing the greeting
      */
     std::string greet(LanguageCode lang = LanguageCode::EN) const;
-
-    /// @brief Return an iso date string
-    std::string getIsoDate() const;
   };
 
 }  // namespace greeter
diff --git a/source/greeter.cpp b/source/greeter.cpp
index 1a5bfb8..11fb416 100644
--- a/source/greeter.cpp
+++ b/source/greeter.cpp
@@ -1,4 +1,4 @@
-#include <fmt/chrono.h>
+#include <fmt/format.h>
 #include <greeter/greeter.h>
 
 using namespace greeter;
@@ -9,17 +9,12 @@ std::string Greeter::greet(LanguageCode lang) const {
   switch (lang) {
     default:
     case LanguageCode::EN:
-      return "Hello, " + name + "!";
+      return fmt::format("Hello, {}!", name);
     case LanguageCode::DE:
-      return "Hallo " + name + "!";
+      return fmt::format("Hallo {}!", name);
     case LanguageCode::ES:
-      return "¡Hola " + name + "!";
+      return fmt::format("¡Hola {}!", name);
     case LanguageCode::FR:
-      return "Bonjour " + name + "!";
+      return fmt::format("Bonjour {}!", name);
   }
 }
-
-std::string Greeter::getIsoDate() const {
-  using namespace std::literals::chrono_literals;
-  return fmt::format("{:%H:%M:%S}", 3h + 15min + 30s);
-}
diff --git a/standalone/source/main.cpp b/standalone/source/main.cpp
index 5464fa5..6874460 100644
--- a/standalone/source/main.cpp
+++ b/standalone/source/main.cpp
@@ -6,15 +6,20 @@
 #include <string>
 #include <unordered_map>
 
-const std::unordered_map<std::string, greeter::LanguageCode> languages{
-    {"en", greeter::LanguageCode::EN},
-    {"de", greeter::LanguageCode::DE},
-    {"es", greeter::LanguageCode::ES},
-    {"fr", greeter::LanguageCode::FR},
-};
-
+// NOLINTNEXTLINE(modernize-use-trailing-return-type)
 int main(int argc, char** argv) {
-  cxxopts::Options options(argv[0], "A program to welcome the world!");
+  // prevent warning: initialization of 'languages' with static storage duration may throw an
+  // exception that cannot be caught [cert-err58-cpp]
+  const std::unordered_map<std::string, greeter::LanguageCode> languages{
+      {"en", greeter::LanguageCode::EN},
+      {"de", greeter::LanguageCode::DE},
+      {"es", greeter::LanguageCode::ES},
+      {"fr", greeter::LanguageCode::FR},
+  };
+
+  // prevent warning: do not use pointer arithmetic
+  // [cppcoreguidelines-pro-bounds-pointer-arithmetic]
+  cxxopts::Options options(*argv, "A program to welcome the world!");
 
   std::string language;
   std::string name;
@@ -33,7 +38,10 @@ int main(int argc, char** argv) {
   if (result["help"].as<bool>()) {
     std::cout << options.help() << std::endl;
     return 0;
-  } else if (result["version"].as<bool>()) {
+  }
+
+  // prevent warning: do not use 'else' after 'return' [readability-else-after-return]
+  if (result["version"].as<bool>()) {
     std::cout << "Greeter, version " << GREETER_VERSION << std::endl;
     return 0;
   }
diff --git a/test/source/greeter.cpp b/test/source/greeter.cpp
index cab989b..10a1242 100644
--- a/test/source/greeter.cpp
+++ b/test/source/greeter.cpp
@@ -24,8 +24,3 @@ TEST_CASE("Greeter version") {
   CHECK(std::string(GREETER_VERSION) == std::string("1.0"));
 #endif
 }
-
-TEST_CASE("Greeter date") {
-  const greeter::Greeter greeter("Tests");
-  CHECK(greeter.getIsoDate() == std::string("03:15:30"));
-}