-
Notifications
You must be signed in to change notification settings - Fork 87
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
feat: add string builder #2899
feat: add string builder #2899
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files |
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.
@lobis - looks great! I don't think clang-format
was run. Could you please run it by hand? Thanks!
By the way I noticed that there is not
I manually formatted the file (no clang-format). I wasn't able to run clang-format without causing a huge diff so I created a separate PR for this (#2902). |
Co-authored-by: Ianna Osborne <ianna.osborne@cern.ch>
dfffb92
to
39289ef
Compare
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.
@lobis - Thanks, it looks good to me.
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.
Great, thanks! I approve it as well.
I'm pretty sure this will automatically pass all of the test (unless there's a C++ compilation error or something).
Is it ready to merge, assuming that it does pass tests?
Ready! |
@all-contributors please add @lobis for code |
I've put up a pull request to add @lobis! 🎉 |
Add a string builder to the header-only library to make building arrays of strings easier.