Limit number of columns to 160#1491
Conversation
|
It looks like we're gonna need to fix all the instances where the columns are long or limit it from CI being the cause of failing. But I'm all for the idea! |
|
... and it seems, I'm using a different clang-format version (21.1.0) than the CI. Is version to use specified, somewhere? |
|
clang-format version is listed here. Not sure if that's up-to-date though. |
|
Looking at the numerous changes: Increasing the column limit might make sense, but some of the changes make code harder to read. Not sure if this is something we want to apply across the board to all samples. |
You mean, even more than 160 columns?
Could you point me at some of those places? Maybe with some additional clang-format settings, it looks better, again.
I think, it should either be done for the complete repository, or not at all. |
ca0d397 to
3eb0a5d
Compare
|
Well, in fact it's clang-format 15.0.6, that is used. And for some unknown reasons, it required a few manual adjustments. But I definitely won't crank up the copyright of all those formatted files. |
06a2b6e to
5ce473a
Compare
4414079 to
13ddd1b
Compare
app/plugins/data_path/data_path.cpp
Outdated
| "Specify the folder containing the sample data folders.", | ||
| {vkb::Hook::OnAppStart}, | ||
| {}, | ||
| DataPathTags("Data Path Override", "Specify the folder containing the sample data folders.", {vkb::Hook::OnAppStart}, {}, |
There was a problem hiding this comment.
I'm not really sure if this improves readability. I actually prefer the way it was written before this change.
There was a problem hiding this comment.
Any way to fix this? There are several spots that have had their wrapping changed in a way that at least IMO hurts code readabilit.
There was a problem hiding this comment.
With BinPackParameters: false, that looks better again.
13ddd1b to
68a2891
Compare
| @@ -1,4 +1,4 @@ | |||
| /* Copyright (c) 2020-2025, Arm Limited and Contributors | |||
| /* Copyright (c) 2020-2026, Arm Limited and Contributors | |||
| * Copyright (c) 2025, NVIDIA CORPORATION. All rights reserved. | |||
There was a problem hiding this comment.
Apparently, neither the copyright.py fixes such Copyright strings, nor the copyright check of the CI catches them?
f6bf7a7 to
0fee7e7
Compare
0fee7e7 to
03bb966
Compare
|
Well, no matter what clang-format version (15.0.x, 0 <= x <= 7 ), win64, I tried, there are four files that needs some manual adjustments to keep the format check happy. |
Description
Every now and then, I'm surprised about the length of some code lines. I would suggest to limit that to, say, 160.
The only manual change in this PR is in
.clang-format, settingColumnLimit : 160. Then all *.cpp and *.h files are formatted accordingly.In addition to the column limit I changed the following option:
BinPackParameters: falseBuild tested on Win11 with VS2022. Run tested on Win11 with NVidia GPU.
General Checklist:
Please ensure the following points are checked:
Note: The Samples CI runs a number of checks including:
If this PR contains framework changes:
batchcommand line argument to make sure all samples still work properlySample Checklist
If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist: