- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 3.2k
 
Add -Wsign-conversion to the list of common warning flags #2775
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
base: devel
Are you sure you want to change the base?
Conversation
249b5f5    to
    f28ab91      
    Compare
  
    
          Codecov Report
 Additional details and impacted files@@            Coverage Diff             @@
##            devel    #2775      +/-   ##
==========================================
- Coverage   91.45%   91.39%   -0.06%     
==========================================
  Files         194      194              
  Lines        8197     8196       -1     
==========================================
- Hits         7496     7490       -6     
- Misses        701      706       +5      | 
    
| std::vector<ColumnInfo> m_columnInfos; | ||
| ReusableStringStream m_oss; | ||
| int m_currentColumn = -1; | ||
| std::size_t m_currentColumn = 0; | 
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.
Would it be better if this was an iterator?
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.
+1
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.
Changed
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.
This is a good improvement and will make a great impact to the project. Thanks @Wmbat for making these changes.. Good Job!!
| std::vector<ColumnInfo> m_columnInfos; | ||
| ReusableStringStream m_oss; | ||
| int m_currentColumn = -1; | ||
| std::size_t m_currentColumn = 0; | 
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.
+1
f28ab91    to
    1d322a4      
    Compare
  
    | 
           Fixed the merge conflicts and switch to using an iterator instead of an index not fully sure why it says I modified so many files though  | 
    
Description
Fixes all implicit sign conversions and turns on -Wsign-conversion when building catch2. I usually tried to find a good solution to the warning instead of casting.
Ran all the tests on my machine and it seemed good to go
GitHub Issues
Related to #2583