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

Converting constructor removal; distinct types; thread-safe time access #118

Merged
merged 5 commits into from
Mar 19, 2024

Conversation

piotr-topnotch
Copy link
Contributor

@piotr-topnotch piotr-topnotch commented Mar 18, 2024

Complete the rework with the following improvements:

  1. Get rid of converting constructors by replacing them with explicit ones.
  2. Make all Response-derived types distinct to allow type-based template dispatching and dynamic type-aware switches (a.k.a. reflections) to work predictably.
  3. std::localtime is not thread-safe. Replaced with localtime_r and localtime_s wrapped within a helper function hiding the OS implementation details.
  4. Provided more explicitly defaulted entities (constructors, assignment operators).
  5. Various minor fixes.

Copy link
Member

@HJLebbink HJLebbink left a comment

Choose a reason for hiding this comment

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

LGTM, I've compiled it with msvc and it works

@HJLebbink HJLebbink force-pushed the feature/hardening-2 branch from a1bdd6b to 621defa Compare March 19, 2024 14:30
@HJLebbink
Copy link
Member

@kobalicek Could you give it a thumbs up.

Copy link
Contributor

@kobalicek kobalicek left a comment

Choose a reason for hiding this comment

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

LGTM

@piotr-topnotch piotr-topnotch merged commit 05a4fc9 into main Mar 19, 2024
3 checks passed
@piotr-topnotch piotr-topnotch deleted the feature/hardening-2 branch March 19, 2024 15:59
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