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

Remove warning disable warning 4800 #841

Closed
wants to merge 1 commit into from

Conversation

AZero13
Copy link
Contributor

@AZero13 AZero13 commented Oct 14, 2022

We should do a comparison with 0 instead of forcing to bool

@mborland
Copy link
Member

What does this change if the warnings are already suppressed? How did you generate/find these?

@AZero13 AZero13 force-pushed the emplace branch 2 times, most recently from c999d96 to e47ecfb Compare October 16, 2022 14:54
@AZero13
Copy link
Contributor Author

AZero13 commented Oct 17, 2022

I found these while browsing the code. It is best to get a bool directly via comparison than a conversion. It is considered to be better C++

We should do a comparison with 0 instead of forcing to bool
@mborland
Copy link
Member

Without a specific bug/error/warning I am going to close this. Generally we are looking for compelling reasons to change code because of our large user base. If you are looking for ways to contribute there is a feature wishlist #303, or scan through our other open issues.

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