merge

.clang-format

@@ -1,4 +1,4 @@
-ColumnLimit: 80
+ColumnLimit: 90
 IndentWidth: 2
 AccessModifierOffset: -1
 
@@ -7,4 +7,4 @@ AllowShortIfStatementsOnASingleLine: true
 AlwaysBreakTemplateDeclarations: true
 BinPackArguments: true
 BinPackParameters: true
-ConstructorInitializerAllOnOneLineOrOnePerLine: false
+ConstructorInitializerAllOnOneLineOrOnePerLine: false

CMakeLists.txt

@@ -35,6 +35,7 @@ option (PORTS_OF_CALL_BUILD_TESTING "Test the current installation")
 
 # CONFIGURATION LOGIC
 # ----------------------------------------
+set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_LIST_DIR}/cmake)
 
 # TARGET CONFIGURATION
 # ----------------------------------------
@@ -61,7 +62,7 @@ target_include_directories(${POCLIB}
 
 target_compile_features(${POCLIB}
   INTERFACE
-    cxx_std_14
+    cxx_std_17
 )
 
 #NOTE this option is done downstream, maybe poc should control it
@@ -80,6 +81,10 @@ if(PORTS_OF_CALL_BUILD_TESTING)
   add_subdirectory(test)
 endif()
 
+# FORMATTING
+# ----------------------------------------
+include(Format)
+
 # INSTALL & EXPORT
 # ----------------------------------------
 

cmake/Format.cmake

@@ -0,0 +1,66 @@
+#=========================================================================================
+# (C) (or copyright) 2024. Triad National Security, LLC. All rights reserved.
+#
+# This program was produced under U.S. Government contract 89233218CNA000001 for Los
+# Alamos National Laboratory (LANL), which is operated by Triad National Security, LLC
+# for the U.S. Department of Energy/National Nuclear Security Administration. All rights
+# in the program are reserved by Triad National Security, LLC, and the U.S. Department
+# of Energy/National Nuclear Security Administration. The Government is granted for
+# itself and others acting on its behalf a nonexclusive, paid-up, irrevocable worldwide
+# license in this material to reproduce, prepare derivative works, distribute copies to
+# the public, perform publicly and display publicly, and to permit others to do so.
+#=========================================================================================
+
+find_program(
+    CLANG_FORMAT
+    NAMES
+        clang-format-12 # Debian package manager, among others, provide this name
+        clang-format-mp-12.0 # MacPorts
+        clang-format # Default name
+    )
+
+if (CLANG_FORMAT AND NOT CLANG_FORMAT_VERSION)
+    # Get clang-format --version
+    execute_process(
+        COMMAND ${CLANG_FORMAT} --version
+        OUTPUT_VARIABLE CLANG_FORMAT_VERSION_OUTPUT)
+
+    if (CLANG_FORMAT_VERSION_OUTPUT MATCHES "clang-format version ([0-9]+\.[0-9]+\.[0-9]+)")
+        set(CLANG_FORMAT_VERSION ${CMAKE_MATCH_1})
+        message(STATUS "clang-format --version: " ${CLANG_FORMAT_VERSION})
+
+        set(CLANG_FORMAT_VERSION ${CLANG_FORMAT_VERSION} CACHE STRING "clang-format version")
+    endif()
+endif()
+
+if (NOT CLANG_FORMAT_VERSION)
+    message(
+        WARNING
+        "Couldn't determine clang-format version. clang-format 12.0 is \
+        expected - results on other versions may not be stable")
+
+    set(CLANG_FORMAT_VERSION "0.0.0" CACHE STRING "clang-format version not found")
+elseif (NOT (CLANG_FORMAT_VERSION VERSION_GREATER_EQUAL "12.0" AND
+         CLANG_FORMAT_VERSION VERSION_LESS "13.0"))
+    message(
+        WARNING
+        "clang-format version 12.0 is required - results on other \
+        versions may not be stable")
+endif()
+
+# Specifically trying to exclude external here - I'm not sure if there's a better way
+set(
+    GLOBS
+    ${PROJECT_SOURCE_DIR}/spiner/[^\.]*.cpp          ${PROJECT_SOURCE_DIR}/spiner/[^\.]*.hpp
+    ${PROJECT_SOURCE_DIR}/ports-of-call/[^\.]*.cpp   ${PROJECT_SOURCE_DIR}/ports-of-call/[^\.]*.hpp
+    ${PROJECT_SOURCE_DIR}/test/[^\.]*.cpp            ${PROJECT_SOURCE_DIR}/test/[^\.]*.hpp
+    ${PROJECT_SOURCE_DIR}/installtest/[^\.]*.cpp     ${PROJECT_SOURCE_DIR}/installtest/[^\.]*.hpp
+)
+
+file(GLOB_RECURSE FORMAT_SOURCES CONFIGURE_DEPENDS ${GLOBS})
+
+if (CLANG_FORMAT)
+  add_custom_target(format_ports
+    COMMAND ${CLANG_FORMAT} -i ${FORMAT_SOURCES}
+    VERBATIM)
+endif()

doc/sphinx/src/developing.rst

@@ -8,3 +8,147 @@ required. We do require the files be formatted using
 ``clang-format``. The format version is pinned 12. Please call
 ```clang-format`` on your files before merging them.
 
+You may do so automatically either by making the ``format_ports``
+target in the build system, e.g.,
+
+.. code-block:: bash
+
+  mkdir -p ports-of-call/build
+  cd ports-of-call/build
+  cmake ..
+  make format_ports
+
+or you may call the ``format.sh`` script in the ``scripts``
+directory. This script supports two environment variables, ``CFM`` and
+``VERBOSE``. ``CFM`` is the path (or name) of your ``clang-format``
+executable. ``VERBOSE=1`` will cause it to output all files it
+modifies.
+
+The docs are built but not deployed on PRs from forks, and the
+internal tests will not be run automatically. So when the code is
+ready for merge, you must ask a project maintainer to trigger the
+remaining tests for you.
+
+Tests
+-------
+
+``ports-of-call`` uses the `Catch2`_ unit testing infrastructure,
+v3. To add a test, add a new ``cpp`` file in the ``test`` directory
+that implements your ``catch2`` scenarios and/or test cases and then
+add it to the ``target_sources`` line at the bottom of
+``test/CMakeLists.txt``.
+
+.. _Catch2: https://github.com/catchorg/Catch2
+
+Expectations for code review
+-----------------------------
+
+From the perspective of the contributor
+````````````````````````````````````````
+
+Code review is an integral part of the development process
+for ``ports-of-call``. You can expect at least one, perhaps many,
+core developers to read your code and offer suggestions.
+You should treat this much like scientific or academic peer review.
+You should listen to suggestions but also feel entitled to push back
+if you believe the suggestions or comments are incorrect or
+are requesting too much effort.
+
+Reviewers may offer conflicting advice, if this is the case, it's an
+opportunity to open a discussion and communally arrive at a good
+approach. You should feel empowered to argue for which of the
+conflicting solutions you prefer or to suggest a compromise. If you
+don't feel strongly, that's fine too, but it's best to say so to keep
+the lines of communication open.
+
+Big contributions may be difficult to review in one piece and you may
+be requested to split your pull request into two or more separate
+contributions. You may also receive many "nitpicky" comments about
+code style or structure. These comments help keep a broad codebase,
+with many contributors uniform in style and maintainable with
+consistent expectations accross the code base. While there is no
+formal style guide for now, the regular contributors have a sense for
+the broad style of the project. You should take these stylistic and
+"nitpicky" suggestions seriously, but you should also feel free to
+push back.
+
+As with any creative endeavor, we put a lot of ourselves into our
+code. It can be painful to receive criticism on your contribution and
+easy to take it personally. While you should resist the urge to take
+offense, it is also partly code reviewer's responsiblity to create a
+constructive environment, as discussed below.
+
+Expectations of code reviewers
+````````````````````````````````
+
+A good code review builds a contribution up, rather than tearing it
+down. Here are a few rules to keep code reviews constructive and
+congenial:
+
+* You should take the time needed to review a contribution and offer
+  meaningful advice. Unless a contribution is very small, limit
+  the times you simply click "approve" with a "looks good to me."
+
+* You should keep your comments constructive. For example, rather than
+  saying "this pattern is bad," try saying "at this point, you may
+  want to try this other pattern."
+
+* Avoid language that can be misconstrued, even if it's common
+  notation in the commnunity. For example, avoid phrases like "code
+  smell."
+
+* Explain why you make a suggestion. In addition to saying "try X
+  instead of Y" explain why you like pattern X more than pattern Y.
+
+* A contributor may push back on your suggestion. Be open to the
+  possibility that you're either asking too much or are incorrect in
+  this instance. Code review is an opportunity for everyone to learn.
+
+* Don't just highlight what you don't like. Also highlight the parts
+  of the pull request you do like and thank the contributor for their
+  effort.
+
+Some notes on style and code architecture
+``````````````````````````````````````````
+
+* A major influence on code style and architecture is the
+  `ten rules for developing safety-critical code`_, by Gerard Holzmann.
+  Safety critical code is code that exists in a context where failure
+  implies serious harm. A flight controler on an airplane or
+  spacecraft or the microcontroller in a car are examples of
+  safety-critical contexts. ``ports-of-call`` is not safety-critical
+  but many of the coding habits advocated for by Holzmann produce
+  long-lived, easy to understand, easy to parse, and easy to maintain code.
+  And we take many of the rules to heart. Here are a few that are most
+  relevant to ``ports-of-call``. They have been adapted slightly to
+  our context.
+
+    #. Avoid complex flow constructs such as gotos.
+
+    #. All loops must have fixed bounds. This prevents runaway
+       code. (Note this implies that as a general rule, one should use
+       ``for`` loops, not ``while`` loops. It also implies one should
+       keep recursion to a minimum.)
+
+    #. Heap memory allocation should only be performed at
+       initialization. Heap memory de-allocation should only be
+       performed at cleanup.
+
+    #. Restrict the length of functions to a single printed page.
+
+    #. Restrict the scope of data to the smallest possible.
+
+    #. Limit pointer use to a single dereference. Avoid pointers of
+       pointers when possible.
+
+    #. Be compiler warning aware. Try to address compiler warnings as
+       they come up.
+
+.. _ten rules for developing safety-critical code: http://web.eecs.umich.edu/~imarkov/10rules.pdf
+
+* ``ports-of-call`` is a modern C++ code
+  and both standard template library capabilities and template
+  metaprogramming are leveraged frequently. This can sometimes make
+  parsing the code difficult. If you see something you don't
+  understand, ask. It may be it can be refactored to be more simple or
+  better documented.