Format & add CI job (#159)

* Add clang-format to workflow * Format pass * Ignore format pass on blame * Add pre-commit clang-format * Add format options to README * Mozilla * Reformat * Reformat * Ensure matching clang-format version * Reformat * Lint and enable new tests
Quicr · Sep 3, 2024 · 700c878 · 700c878
1 parent fd05bd9
commit 700c878
Show file tree

Hide file tree

Showing 25 changed files with 2,340 additions and 2,206 deletions.
diff --git a/.clang-format b/.clang-format
@@ -7,4 +7,4 @@ ColumnLimit: 120
 IndentWidth: 4
 NamespaceIndentation: All
 BraceWrapping:
-  AfterFunction: false
+  AfterFunction: false
diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs
@@ -1,2 +1,3 @@
 # Rename
 b42d453d603e54e395efe9b537f9687cc33b4315
+a90f00c6f9d1f59293dfa216b4735e9d3e9053b9
diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml
@@ -11,6 +11,15 @@ env:
   BUILD_TYPE: Release
 
 jobs:
+  format-check:
+    name: Format Check
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: jidicula/[email protected]
+        with:
+          clang-format-version: '18'
+
   build:
     # The CMake configure and build commands are platform agnostic and should work equally well on Windows or Mac.
     # You can convert this to a matrix build if you need cross-platform coverage.

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -0,0 +1,5 @@
+repos:
+- repo: https://github.com/pre-commit/mirrors-clang-format
+  rev: v18.1.8
+  hooks:
+  - id: clang-format
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -88,6 +88,7 @@ endif()
 ###
 
 if(BUILD_TESTING AND QTRANSPORT_BUILD_TESTS)
+  enable_testing()
   add_subdirectory(cmd)
   add_subdirectory(test)
 endif()
diff --git a/Makefile b/Makefile
@@ -28,6 +28,7 @@ cclean:
 	rm -rf ${BUILD_DIR}
 
 format:
-	find include -iname "*.h" -or -iname "*.cpp" | xargs ${CLANG_FORMAT}
-	find src -iname "*.h" -or -iname "*.cpp" | xargs ${CLANG_FORMAT}
-	find cmd -iname "*.h" -or -iname "*.cpp" | xargs ${CLANG_FORMAT}
+	find include -iname "*.h" -or -iname "*.cpp" -or -iname "*.cc" | xargs ${CLANG_FORMAT}
+	find src -iname "*.h" -or -iname "*.cpp" -or -iname "*.cc" | xargs ${CLANG_FORMAT}
+	find cmd -iname "*.h" -or -iname "*.cpp" -or -iname "*.cc" | xargs ${CLANG_FORMAT}
+	find test -iname "*.h" -or -iname "*.cpp" -or -iname "*.cc" | xargs ${CLANG_FORMAT}
diff --git a/README.md b/README.md
@@ -51,7 +51,11 @@ RELAY_HOST=localhost RELAY_PORT=1234 build/cmd/client
 
 ## Development
 
-This project generally follows Google's C++ styling, specifically enforced for naming and formatting: https://google.github.io/styleguide/cppguide.html
+This project generally follows Google's and Mozilla's C++ styling, specifically enforced for naming and formatting: https://google.github.io/styleguide/cppguide.html
+
+Linting via clang-tidy can be enabled via passing the `-DLINT=ON` option to CMake, which is what CI will enforce.
+
+`clang-format` can be run be run by running `make format`. CI will enforce this. A pre-commit file for clang-format is provided, which you can install using `pre-commit install`.
 
 # Notes
 

diff --git a/cmd/client.cc b/cmd/client.cc
@@ -3,9 +3,9 @@
 #include <sstream>
 #include <thread>
 
-#include <transport/transport.h>
-#include <spdlog/spdlog.h>
 #include <spdlog/sinks/stdout_color_sinks.h>
+#include <spdlog/spdlog.h>
+#include <transport/transport.h>
 
 #include "object.h"
 
@@ -20,7 +20,7 @@ struct Delegate : public ITransport::TransportDelegate
     std::shared_ptr<ITransport> client;
     TransportConnId conn_id;
     std::shared_ptr<spdlog::logger> logger;
-    Object _rx_object{logger};
+    Object _rx_object{ logger };
 
   public:
     Delegate()
@@ -29,9 +29,7 @@ struct Delegate : public ITransport::TransportDelegate
         conn_id = 0;
     }
 
-    void stop() {
-        client.reset();
-    }
+    void stop() { client.reset(); }
 
     void setClientTransport(std::shared_ptr<ITransport> client) { this->client = client; }
 
@@ -42,12 +40,12 @@ struct Delegate : public ITransport::TransportDelegate
         SPDLOG_LOGGER_INFO(logger, "Connection state change conn_id: {0}, {1}", conn_id, int(status));
     }
 
-    void OnNewConnection(const TransportConnId& , const TransportRemote&) {}
+    void OnNewConnection(const TransportConnId&, const TransportRemote&) {}
 
     void OnRecvStream(const TransportConnId& conn_id,
-                        uint64_t stream_id,
-                        std::optional<DataContextId> data_ctx_id,
-                        [[maybe_unused]] const bool is_bidir)
+                      uint64_t stream_id,
+                      std::optional<DataContextId> data_ctx_id,
+                      [[maybe_unused]] const bool is_bidir)
     {
         auto stream_buf = client->GetStreamBuffer(conn_id, stream_id);
 
@@ -70,10 +68,9 @@ struct Delegate : public ITransport::TransportDelegate
         }
     }
 
-    void OnRecvDgram(const TransportConnId& conn_id,
-                       std::optional<DataContextId> data_ctx_id)
+    void OnRecvDgram(const TransportConnId& conn_id, std::optional<DataContextId> data_ctx_id)
     {
-        for (int i=0; i < 50; i++) {
+        for (int i = 0; i < 50; i++) {
             auto data = client->Dequeue(conn_id, data_ctx_id);
 
             if (data) {
@@ -137,7 +134,9 @@ main()
 
     int period_count = 0;
 
-    ITransport::EnqueueFlags encode_flags { .use_reliable = use_reliable, .new_stream = true, .clear_tx_queue = true, .use_reset = true};
+    ITransport::EnqueueFlags encode_flags{
+        .use_reliable = use_reliable, .new_stream = true, .clear_tx_queue = true, .use_reset = true
+    };
 
     auto tx_object = Object(logger);
 
@@ -147,8 +146,9 @@ main()
             auto obj = tx_object.encode();
 
             std::vector<MethodTraceItem> trace;
-            const auto start_time = std::chrono::time_point_cast<std::chrono::microseconds>(std::chrono::steady_clock::now());
-            trace.push_back({"client:publish", start_time});
+            const auto start_time =
+              std::chrono::time_point_cast<std::chrono::microseconds>(std::chrono::steady_clock::now());
+            trace.push_back({ "client:publish", start_time });
 
             if (encode_flags.use_reliable) {
                 if (period_count > 2000) {
@@ -163,24 +163,15 @@ main()
                 }
             }
 
-            client->Enqueue(conn_id,
-                            data_ctx_id,
-                            std::move(obj),
-                            std::move(trace),
-                            1,
-                            350,
-                            0,
-                            encode_flags);
+            client->Enqueue(conn_id, data_ctx_id, std::move(obj), std::move(trace), 1, 350, 0, encode_flags);
         }
 
         // Increase delay if using UDP, need to pace more
         if (server.proto == TransportProtocol::kUdp) {
-            std::this_thread::sleep_for(std::chrono::milliseconds (10));
+            std::this_thread::sleep_for(std::chrono::milliseconds(10));
         } else {
             std::this_thread::sleep_for(std::chrono::milliseconds(2));
         }
-
-
     }
 
     client->DeleteDataContext(conn_id, data_ctx_id);

diff --git a/cmd/echoServer.cc b/cmd/echoServer.cc
@@ -3,9 +3,9 @@
 #include <sstream>
 #include <thread>
 
-#include <transport/transport.h>
-#include <spdlog/spdlog.h>
 #include <spdlog/sinks/stdout_color_sinks.h>
+#include <spdlog/spdlog.h>
+#include <transport/transport.h>
 
 #include "object.h"
 
@@ -17,7 +17,7 @@ struct Delegate : public ITransport::TransportDelegate
     std::shared_ptr<ITransport> server;
     std::shared_ptr<spdlog::logger> logger;
 
-    Object _object{logger};
+    Object _object{ logger };
 
     DataContextId out_data_ctx{ 0 };
 
@@ -43,15 +43,14 @@ struct Delegate : public ITransport::TransportDelegate
         out_data_ctx = this->server->CreateDataContext(conn_id, true, 10);
     }
 
-
     void OnRecvStream(const TransportConnId& conn_id,
-                        uint64_t stream_id,
-                        std::optional<DataContextId> data_ctx_id,
-                        [[maybe_unused]] const bool is_bidir)
+                      uint64_t stream_id,
+                      std::optional<DataContextId> data_ctx_id,
+                      [[maybe_unused]] const bool is_bidir)
     {
         auto stream_buf = server->GetStreamBuffer(conn_id, stream_id);
 
-        while(true) {
+        while (true) {
             if (stream_buf->Available(4)) {
                 auto len_b = stream_buf->Front(4);
                 if (!len_b.size())
@@ -65,7 +64,14 @@ struct Delegate : public ITransport::TransportDelegate
 
                     _object.process(conn_id, data_ctx_id, obj);
 
-                    server->Enqueue(conn_id, out_data_ctx, std::move(obj), { MethodTraceItem{} }, 2, 500, 0, { true, false, false, false });
+                    server->Enqueue(conn_id,
+                                    out_data_ctx,
+                                    std::move(obj),
+                                    { MethodTraceItem{} },
+                                    2,
+                                    500,
+                                    0,
+                                    { true, false, false, false });
                 } else {
                     break;
                 }
@@ -75,10 +81,9 @@ struct Delegate : public ITransport::TransportDelegate
         }
     }
 
-    void OnRecvDgram(const TransportConnId& conn_id,
-                       std::optional<DataContextId> data_ctx_id)
+    void OnRecvDgram(const TransportConnId& conn_id, std::optional<DataContextId> data_ctx_id)
     {
-        for (int i=0; i < 150; i++) {
+        for (int i = 0; i < 150; i++) {
             auto data = server->Dequeue(conn_id, data_ctx_id);
 
             if (data) {

diff --git a/cmd/object.cpp b/cmd/object.cpp
@@ -1,6 +1,8 @@
 #include "object.h"
 
-void Object::process(TransportConnId conn_id, std::optional<DataContextId> data_ctx_id, std::vector<uint8_t>& obj) {
+void
+Object::process(TransportConnId conn_id, std::optional<DataContextId> data_ctx_id, std::vector<uint8_t>& obj)
+{
     msgcount++;
 
     if (msgcount % 2000 == 0 && prev_msgcount != msgcount) {
@@ -44,7 +46,9 @@ void Object::process(TransportConnId conn_id, std::optional<DataContextId> data_
     prev_msg_num = *msg_num;
 }
 
-std::vector<uint8_t> Object::encode() {
+std::vector<uint8_t>
+Object::encode()
+{
     std::vector<uint8_t> obj(1000, 0);
 
     uint32_t* len = (uint32_t*)obj.data();

diff --git a/cmd/object.h b/cmd/object.h
@@ -1,16 +1,18 @@
 #pragma once
 
-#include <transport/transport.h>
-#include <spdlog/spdlog.h>
 #include <spdlog/sinks/stdout_color_sinks.h>
+#include <spdlog/spdlog.h>
+#include <transport/transport.h>
 
 using namespace qtransport;
 
 class Object
 {
   public:
-    Object(std::shared_ptr<spdlog::logger> log) :
-      logger(std::move(log)) {}
+    Object(std::shared_ptr<spdlog::logger> log)
+      : logger(std::move(log))
+    {
+    }
 
     void process(TransportConnId conn_id, std::optional<DataContextId> data_ctx_id, std::vector<uint8_t>& obj);
     std::vector<uint8_t> encode();
@@ -20,6 +22,6 @@ class Object
 
     uint64_t msgcount{ 0 };
     uint64_t prev_msgcount{ 0 };
-    uint64_t msg_num { 0 };
+    uint64_t msg_num{ 0 };
     uint32_t prev_msg_num{ 0 };
 };
diff --git a/include/transport/priority_queue.h b/include/transport/priority_queue.h
@@ -1,11 +1,11 @@
 #pragma once
 
 #include <algorithm>
+#include <array>
 #include <chrono>
 #include <forward_list>
 #include <numeric>
 #include <optional>
-#include <array>
 
 #include "time_queue.h"
 
@@ -39,8 +39,7 @@ namespace qtransport {
         };
 
       public:
-        ~PriorityQueue() {
-        }
+        ~PriorityQueue() {}
 
         /**
          * Construct a priority queue
@@ -60,9 +59,9 @@ namespace qtransport {
          * @param initial_queue_size    Number of default fifo queue size (reserve)
          */
         PriorityQueue(size_t duration,
-                       size_t interval,
-                       const std::shared_ptr<TickService>& tick_service,
-                       size_t initial_queue_size)
+                      size_t interval,
+                      const std::shared_ptr<TickService>& tick_service,
+                      size_t initial_queue_size)
           : tick_service_(tick_service)
         {
 
@@ -83,7 +82,7 @@ namespace qtransport {
          * @param priority  The priority of the value (range is 0 - PMAX)
          * @param delay_ttl Delay POP by this ttl value in milliseconds
          */
-        void Push(DataType& value, uint32_t ttl, uint8_t priority = 0, uint32_t delay_ttl=0)
+        void Push(DataType& value, uint32_t ttl, uint8_t priority = 0, uint32_t delay_ttl = 0)
         {
             std::lock_guard<std::mutex> _(mutex_);
 
@@ -99,7 +98,8 @@ namespace qtransport {
          * @param priority  The priority of the value (range is 0 - PMAX)
          * @param delay_ttl Delay POP by this ttl value in milliseconds
          */
-        void Push(DataType&& value, uint32_t ttl, uint8_t priority = 0, uint32_t delay_ttl=0) {
+        void Push(DataType&& value, uint32_t ttl, uint8_t priority = 0, uint32_t delay_ttl = 0)
+        {
             std::lock_guard<std::mutex> _(mutex_);
 
             auto& queue = GetQueueByPriority(priority);
@@ -160,7 +160,7 @@ namespace qtransport {
         /**
          * @brief Clear queue
          */
-         void Clear()
+        void Clear()
         {
             std::lock_guard<std::mutex> _(mutex_);
 
@@ -204,7 +204,8 @@ namespace qtransport {
             }
 
             if (!queue_[priority]) {
-                queue_[priority] = std::make_unique<TimeQueueType>(duration_ms_, interval_ms_, tick_service_, initial_queue_size_);
+                queue_[priority] =
+                  std::make_unique<TimeQueueType>(duration_ms_, interval_ms_, tick_service_, initial_queue_size_);
             }
 
             return queue_[priority];