Skip to content

Conversation

@wusatosi
Copy link
Member

Adds an example that shows generating a fibonacci sequence as a example.

Packed a very partial size fix to let the example compile.

Copy link
Member

@DeveloperPaul123 DeveloperPaul123 left a comment

Choose a reason for hiding this comment

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

Overall looks good, just some minor comments.

Regarding the examples in general, are we mandating that they build with all the supported C++ standards that inplace_vector supports?

@wusatosi
Copy link
Member Author

Overall looks good, just some minor comments.

Regarding the examples in general, are we mandating that they build with all the supported C++ standards that inplace_vector supports?

Thanks for the review, we will eventually add more constexpr examples, e.g. fib with cache for i < 100 computed at compile time or smt. We will need to selectively enable it as it would not be supported on C++ 17 (unless we figure out a way to support constexpr on 17).

Example of this being done in exemplar: bemanproject/exemplar#56

@DeveloperPaul123
Copy link
Member

Overall looks good, just some minor comments.
Regarding the examples in general, are we mandating that they build with all the supported C++ standards that inplace_vector supports?

Thanks for the review, we will eventually add more constexpr examples, e.g. fib with cache for i < 100 computed at compile time or smt. We will need to selectively enable it as it would not be supported on C++ 17 (unless we figure out a way to support constexpr on 17).

Example of this being done in exemplar: bemanproject/exemplar#56

Ok sounds good, thanks for the link. I have missed the last few sync calls so I'm a bit behind. I only asked because it would be nice to use std::print() instead of iostreams in the example(s). But that's just being nitpicky IMO. I don't have an issue landing this PR as is.

@wusatosi
Copy link
Member Author

[use std::print instead of iostream]

Don't forget we are still on C++20/17 for this library :P

@wusatosi
Copy link
Member Author

Thanks for the review, merging...

@wusatosi wusatosi merged commit 8f6435b into bemanproject:main Nov 20, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants