Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update README Eigen version and add {fmt} dependency #254

Closed
wants to merge 4 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ Benchmarks are pushed to the [GitHub pages](https://open-space-collective.github
| ----------- | --------- | ------------------ | ------------------------------------------------------------------------------------------------------------------------------------------ |
| Pybind11 | `2.10.1` | BSD-3-Clause | [github.com/pybind/pybind11](https://github.com/pybind/pybind11) |
| ordered-map | `0.6.0` | MIT | [github.com/Tessil/ordered-map](https://github.com/Tessil/ordered-map) |
| Eigen | `3.3.7` | MPL2 | [eigen.tuxfamily.org](http://eigen.tuxfamily.org/index.php) |
| {fmt} | `5.2.0` | BSD-2-Clause | [github.com/fmtlib/fmt](https://github.com/fmtlib/fmt) |
Copy link
Contributor

@lucas-bremond lucas-bremond Oct 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating! Actually, fmt is already listed as an OSTk Core dependency this is why it isn't duplicated here.

However, the Eigen version bump is valid, thanks for spotting that!

Copy link
Author

@DhruvJ22 DhruvJ22 Oct 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your comment!

OTSK-physics and OTSK-mathematics lists core and {fmt} as a dependency, hence I suggested the change to be consistent with other repos.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, that's likely an oversight! Let's summon @Derollez and @robinpdm to see what they think.

Copy link
Contributor

@robinpdm robinpdm Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. If we want these lists to match the Dockerfiles, then indeed we may want to add {fmt} here? Or should they rather match CMakeLists?

| Eigen | `3.4.0` | MPL2 | [eigen.tuxfamily.org](http://eigen.tuxfamily.org/index.php) |
| SGP4 | `6a448b4` | Apache License 2.0 | [github.com/dnwrnr/sgp4](https://github.com/dnwrnr/sgp4) |
| NLopt | `2.5.0` | LGPL | [github.com/stevengj/nlopt](https://github.com/stevengj/nlopt) |
| benchmark | `1.8.2` | Apache License 2.0 | [github.com/google/benchmark](https://github.com/google/benchmark) |
Expand All @@ -238,7 +239,7 @@ Benchmarks are pushed to the [GitHub pages](https://open-space-collective.github

Contributions are more than welcome!

Please read our [contributing guide](CONTRIBUTING.md) to learn about our development process, how to propose fixes and improvements, and how to build and test the code.
Please read our [contributing guide](https://github.com/open-space-collective/open-space-toolkit-astrodynamics/blob/main/CONTRIBUTING.md) to learn about our development process, how to propose fixes and improvements, and how to build and test the code.

## Special Thanks

Expand Down