-
-
Notifications
You must be signed in to change notification settings - Fork 307
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
Fix compiler warnings, part 12 #1406
Conversation
I found the cause of that problem and addressed it with e673ee7. See: Code Scanning Results I closed the issues (as resolved) where I updated with your suggestions. There remain some open, which need another round. |
Nice catch, obvious now that I see it, not obvious to find, great! |
Addresses -Wformat-overflow= warnings.
Addresses failure to list files with long names and -Wformat-overflow= warnings.
2f4fda4
to
4e9e90b
Compare
Updated with forced push for cleaner commit history.
|
Addresses -Wformat-overflow= warnings.
Addresses failure to list files with long names and -Wformat-overflow= warnings.
9fef853
to
01d49cd
Compare
Addresses failure to list files with long names and -Wformat-overflow= warnings.
Fixes -Wformat compiler warning: <format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘pdal::Dimension::Id’>
01d49cd
to
a9e5645
Compare
Any further thoughts on this PR? There are some (still open) discussions which I personally believe to be resolved. |
@wenzeslaus It would be preferable to keep the commits of this PR separate, as I did with previous similar PRs (cherry pick to local master and push), as they are not really related (only grouped together to simplify review). Do you have any suggestion as to how to proceed? |
The first and last commit are so small that they could be one commit if needed. The overflow part in the two middle ones seems related to each other to me, so putting these two together would be okay. However, the i.group name ellipses changes seem to require a separate PR for the i.group commit.
Direct pushes to the master branch are no longer possible without changing the settings. I have disabled those to avoid accidental pushes when people (with write access) confuse their remotes. (This is now set for both this and addons repo.)
Whatever were the reasons to start with one PR...
...splitting this into multiple PRs seems to be the most straightforward way of going forward. No special merging procedures needed in that case. Likely you know about it already, but if you are seeking a simple way for doing PRs, consider GitHub CLI ( |
Addresses -Wformat-overflow= warnings.
Fixes -Wformat compiler warning: <format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘pdal::Dimension::Id’>
Addresses failure to list files with long names and -Wformat-overflow= warnings.
a9e5645
to
02b70bf
Compare
@wenzeslaus If you are good with latest changes I will commit them through separate PRs. |
Yes, that sounds great! |
Addresses failure to list files with long names and -Wformat-overflow= warnings.
Fixes -Wformat compiler warning: <format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘pdal::Dimension::Id’>
Addresses -Wformat-overflow= warnings.
02b70bf
to
f633ab5
Compare
Addresses -Wformat-overflow= warnings.
f633ab5
to
8e31d41
Compare
Addresses -Wformat-overflow= warnings.
Addresses -Wformat-overflow= warnings.
Fixes compiler warnings:
Twelfth part addressing #1247.
Modules / code parts directly affected: