Skip to content

Conversation

@alippai
Copy link
Contributor

@alippai alippai commented Feb 25, 2025

I have no experience with migrations like this. The tests pass, but it definitely needs a second pair of eyes.

@alippai alippai requested a review from xhochy as a code owner February 25, 2025 04:03
@alippai alippai force-pushed the modern-cpp branch 2 times, most recently from bd133a9 to a91a0a0 Compare February 25, 2025 04:18
@alippai alippai changed the title Remove boost Replace boost with C++20 Feb 25, 2025
Copy link

@bgemmill bgemmill left a comment

Choose a reason for hiding this comment

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

Generally looks good to me, I had a few nits around nanosecond handling and epoch encocing, and it looks like all of this may just require cpp17 rather than 20.

Thanks for looping me in from the issue!

@alippai
Copy link
Contributor Author

alippai commented Feb 28, 2025

@bgemmill thanks for the help. I did my best to address the comments.

@xhochy
Copy link
Collaborator

xhochy commented Mar 4, 2025

Thanks! This looks really promising. Could you try and do some kind of performance test? The last time I removed some boost code the performance got worse and we needed to introduce simdutf as an additional dependency.

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.

4 participants