-
Notifications
You must be signed in to change notification settings - Fork 4
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
Rename phasorinc, add {C,D}Collection documentation, and main()
→ main.cpp
#200
Conversation
Typo in the README instructions. And try to get the details tags working.
Based on discussion with Peter from a few weeks ago.
<details> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
Although I noticed that the current README usage instructions are also somewhat out of date (they don't mention the -h
, -m
or -fd
flags, for example).
Maybe we should look to update this file? Or we could hold off till hdf5
has replaced MATLAB.
Good point, let's fix that here too whilst I'm in a documentationey mood. (It's better to have the documentation change along with changes to the code. ... Implying the doc was there beforehand 😹 ) |
Because the official way to say it is shouting. https://uk.mathworks.com/products/matlab.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks OK, although has the -fd
option disappeared? It's no longer in the documentation you've written. If it's deliberate ignore my other comments.
It looks like the tdms_tests
executable is failing to build too since the GLOB
update - I can't see anything wrong with the CMakeLists
file though, so maybe there's something in targets.cmake
that needs updating?
src/utils.cpp | ||
matlabio/matlabio.cpp | ||
) | ||
file(GLOB_RECURSE SOURCES "src/*.cpp") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests fail because I (stupidly) didn't check the tests target. And this regex also finds main.cpp
so there's a clash between the catch2 main()
and the tdms main()
.
Co-authored-by: Will Graham <[email protected]>
…/TDMS into rename-phasorinc-and-add-doc
There was a hanging documentation and a renaming task from our last meeting with Peter.
I also include a few minor sanity fixes:
main
intomain.cpp
.<details>
tags (or at least it's fixed with my local doxygen version).