cpp branch code review

[mmore500]

Some miscellaneous comments

[mmore500]

https://github.com/mmore500/downstream/blob/10e1ada5dd9aefc1fb0848756a15f6b6916d0504/downstream/dstream/steady_algo/_steady_assign_storage_site.hpp#L7

I wonder if this the global downstream namespace should be shorted to dwst. I guess users can do a using namespace dstream_steady = downstream::dstream::steady_algo, which we might want to model or suggest somehow. Or provide an namespace_alias.hpp file with some short names/shortcuts. Probably the 2nd option is better than changing the global namespace.

[mmore500]

Add a blank line between headerguards and includes throughout https://github.com/mmore500/downstream/blob/10e1ada5dd9aefc1fb0848756a15f6b6916d0504/downstream/dstream/steady_algo/_steady_assign_storage_site.hpp#L2

[mmore500]

We should make a pass throughout to make everything uint64_t and const all the values than can be consted.

https://github.com/mmore500/downstream/blob/10e1ada5dd9aefc1fb0848756a15f6b6916d0504/downstream/dstream/steady_algo/_steady_assign_storage_site.hpp#L13

[mmore500]

(including return types)

[mmore500]

https://github.com/mmore500/downstream/blob/10e1ada5dd9aefc1fb0848756a15f6b6916d0504/downstream/dstream/steady_algo/_steady_assign_storage_site.hpp#L16

Let's factor this (and other functions) into a "private" function that returns a uint64_t that begins with an underscore _steady_assign_storage_site where the return value is S for nullopt values and then the public function that replaces that placeholder value with std::nullopt.

[mmore500]

ha! didn't know this was in the standard. Nice

https://github.com/mmore500/downstream/blob/10e1ada5dd9aefc1fb0848756a15f6b6916d0504/downstream/dstream/steady_algo/_steady_get_ingest_capacity.hpp#L14

[mmore500]

Check header guard

https://github.com/mmore500/downstream/blob/10e1ada5dd9aefc1fb0848756a15f6b6916d0504/downstream/dstream/steady_algo/_steady_get_ingest_capacity.hpp#L1

[mmore500]

Let's change the interface a little. Let's have this return a uint64_t, with it returning std::numeric_limits<uint64_t>::max() if the calculated value overflows (or an infinite value is possible). We can add a comment directing the users to the Python implementation for a more exact implementation that differentiates between infinite and finite but larger than uint64_t.

https://github.com/mmore500/downstream/blob/10e1ada5dd9aefc1fb0848756a15f6b6916d0504/downstream/dstream/steady_algo/_steady_get_ingest_capacity.hpp#L13

[mmore500]

And then we can add this get_ingest_capacity function back in for all algorithms

[mmore500]

Make a pass to wrap lines to max 80 characters

https://github.com/mmore500/downstream/blob/10e1ada5dd9aefc1fb0848756a15f6b6916d0504/downstream/dstream/stretched_algo/_stretched_assign_storage_site.hpp#L16

[mmore500]

https://github.com/mmore500/downstream/blob/10e1ada5dd9aefc1fb0848756a15f6b6916d0504/test_downstream/test_dstream/test_steady_algo/test_steady_assign_storage_site.cpp#L3

Let's use the csignal header https://en.cppreference.com/w/cpp/header/csignal and move the standard header includes above the local includes

[mmore500]

Let's put the headers inside a top-level include/ folder (so, include/downstream/...)

[mmore500]

Let's copy the docstrings over from Python and put them as doxygen style comments (llm might be good for this) https://developer.lsst.io/cpp/api-docs.html

[mmore500]

Nice work --- using clang-format is much more sane (can we configure a max line width?) https://github.com/mmore500/downstream/blob/cpp/style.sh

[mmore500]

Let's consolidate everything in here into one top-level main.cpp

https://github.com/mmore500/downstream/tree/cpp/test_downstream/test_dstream

[mmore500]

Let's rename the ci testing job test-validators for consistency with python impl and rename this file as cpp-ci.yaml (also for consistency) https://github.com/mmore500/downstream/blob/cpp/.github/workflows/ci.yaml

[mmore500]

We'll also need to update the badge on the cpp branch README to point to cpp-ci

[mmore500]

Let’s add the cpp branch as a submodule on the main branch now that we have something going

[mmore500]

We should add version information that the end user can access somehow. Not sure what best practice is for this

[mmore500]

makefile looks good!