-
Notifications
You must be signed in to change notification settings - Fork 36
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
Question: language standard, pull requests. #213
Comments
I would point out a possible bug: The declaration in the header assigned -1 to the unsigned char, which should underflow and give the max value afaik. I made some changes in a fork of the repo that address that by adding the NOMINMAX define before including Windows.h, and then replacing the one single instance of the macros' usage with std::min and std::max. https://github.com/calebtt/xOBSE/tree/modernized There are other changes included in the commit, such as changing static const integral variables to static constexpr, a few instances of changing std::container_type<element_type>::iterator to "auto", and other things of that nature--replaced NULL with nullptr. Changed a call to .size() to determine if empty to !.empty(), etc. |
Hi @calebtt the current standard is iirc setted as c++latest that should contains c++17 standard with additions from c++20 standard. I don't care what standard is used, until is supported by vs2019 toolchains, and I ask to not bumb to vs2022 as the ide and its tolchains aren't usable under wine. For conding standard I'm not sure, I'd like to be consistent. I will provide direct feedback in the pull request analysing cade by case. |
Hi @calebtt |
Yes that looks correct, I'm sorry I didn't look into it earlier. Lots of work work. |
I would like to contribute some code to this project, but I have a couple of questions:
1. What C++ language standard are you targeting?
2. Any advice on pull requests?
3. Coding standards? C++ core guidelines would be great to follow if you don't have your own.
I don't intend to make major architectural modifications anytime soon, just some code quality and modernization changes, if the language standard will support that.
The text was updated successfully, but these errors were encountered: