-
-
Notifications
You must be signed in to change notification settings - Fork 78
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
CI: bump clang-format to 20 #1157
Conversation
WalkthroughThis pull request updates the GitHub Actions workflow to install Clang tools version 20 instead of version 19 and applies widespread code reformatting changes across the codebase. The changes include modifications to indentation, line breaks, and multi-line splits in method signatures and calls. All adjustments are cosmetic; they enhance readability without affecting the underlying logic, control flow, or functionality of the application. Changes
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Cabin.cc (1)
85-94
: Pass-by-value Usage in colorizeAnyhowErrorAlthough static analysis hints suggest that a function parameter might be passed by const reference, the current implementation of
colorizeAnyhowError
deliberately accepts its string parameter by value to allow for move semantics during modification. This is an acceptable and common pattern; however, please keep an eye on performance if this function is called with very large strings.🧰 Tools
🪛 Cppcheck (2.10-2)
[performance] 94-94: Function parameter 'other' should be passed by const reference.
(passedByValue)
src/Manifest.cc (1)
30-48
: Optional Improvement: Parameter Passing intryFromString
.
The functionEdition::tryFromString(std::string str)
currently takes its parameter by value. Although this allows for move semantics later, static analysis suggests that taking the parameter as aconst std::string&
might be more efficient when the argument does not need to be mutated. Since this PR focuses solely on formatting, you may consider this change in a separate refactoring if performance becomes a concern.🧰 Tools
🪛 Cppcheck (2.10-2)
[performance] 43-43: Function parameter 'macros' should be passed by const reference.
(passedByValue)
[performance] 43-43: Function parameter 'includeDirs' should be passed by const reference.
(passedByValue)
[performance] 44-44: Function parameter 'others' should be passed by const reference.
(passedByValue)
[performance] 38-38: Function parameter 'other' should be passed by const reference.
(passedByValue)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.github/workflows/cpp.yml
(1 hunks)src/Algos.hpp
(1 hunks)src/Builder/Project.cc
(1 hunks)src/Cabin.cc
(1 hunks)src/Cli.cc
(4 hunks)src/Cli.hpp
(2 hunks)src/Cmd/Add.cc
(1 hunks)src/Cmd/Build.cc
(1 hunks)src/Cmd/Clean.cc
(1 hunks)src/Cmd/Fmt.cc
(1 hunks)src/Cmd/Lint.cc
(1 hunks)src/Cmd/Remove.cc
(1 hunks)src/Cmd/Run.cc
(1 hunks)src/Cmd/Search.cc
(1 hunks)src/Command.cc
(2 hunks)src/Manifest.cc
(3 hunks)src/Parallelism.cc
(1 hunks)src/Rustify/Tests.hpp
(3 hunks)src/TermColor.hpp
(1 hunks)
🧰 Additional context used
🪛 Cppcheck (2.10-2)
src/Cabin.cc
[performance] 25-25: Function parameter 'str' should be passed by const reference.
(passedByValue)
[performance] 38-38: Function parameter 'other' should be passed by const reference.
(passedByValue)
src/Manifest.cc
[performance] 58-58: Function parameter 'str' should be passed by const reference.
(passedByValue)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: build & test (Clang 16)
- GitHub Check: build & test (GCC 13)
- GitHub Check: build & test (Clang 18)
- GitHub Check: build & test (Apple Clang - macOS 13)
- GitHub Check: build & test (GCC 12)
- GitHub Check: build & test (GCC 14)
- GitHub Check: build & test (Clang 17)
- GitHub Check: build & test (Clang 19)
- GitHub Check: analyze (c-cpp)
- GitHub Check: create
🔇 Additional comments (28)
src/Parallelism.cc (1)
53-57
: Cosmetic Reformat in Constructor Initializer List
The initializer list in theParallelismState
constructor has been reformatted into multiple lines for enhanced readability. There is no change in logic or functionality.src/TermColor.hpp (1)
31-34
: Improvedrequires
Clause Formatting
Therequires
clause for the templatedColorStr
constructor has been split into multiple lines to improve readability without affecting its behavior..github/workflows/cpp.yml (2)
140-141
: Upgrade to Clang Tools Version 20
The commands have been updated to install Clang tools version 20 (usingsudo ./llvm.sh 20
and installingclang-format-20
), aligning with the PR objective.
146-146
: Updated Environment Variable for Clang-Format
The environment variableCABIN_FMT
now referencesclang-format-20
, ensuring consistency with the new installation commands.src/Rustify/Tests.hpp (3)
155-159
: Refined Formatting inassertEq
Error Message
The formatting string in theassertEq
function has been reformatted into a multi-line structure to enhance clarity. This change is purely cosmetic and does not alter the assertion logic.
194-198
: Clearer Formatting inassertNe
Error Message
The error message in theassertNe
function now uses a multi-line format, making the output easier to read while retaining its original semantics.
232-237
: Enhanced Readability inassertLt
Error Message
The multi-line formatting of the error message inassertLt
improves readability without impacting functionality.src/Cmd/Remove.cc (1)
23-28
: Reformatted Argument Declaration forREMOVE_CMD
The argument for the"deps"
parameter has been spread across multiple lines for improved clarity. This formatting refinement does not change the underlying logic.src/Builder/Project.cc (1)
104-106
: Improved Compiler Flag FormattingThe new multi-line layout for appending the optimization flag (using
fmt::format("-O{}", profile.optLevel)
) improves the readability of the code. No functionality has been altered.src/Cmd/Fmt.cc (1)
29-33
: CLI Option Formatting Enhancement:--exclude
Reformatting the definition of the
--exclude
option into a multi-line structure enhances clarity without changing its behavior.src/Cmd/Build.cc (1)
32-36
: CLI Command Option Reformatting for--compdb
Splitting the
--compdb
option declaration into multiple lines improves readability and is consistent with the new formatting style applied throughout the project. This cosmetic change does not impact functionality.src/Cmd/Search.cc (1)
23-34
: Improved Readability for Search Command OptionsExpanding the option definitions for
--per-page
and--page
across multiple lines clearly separates their properties (description, placeholder, and default value), thereby enhancing code understandability while preserving their intended behavior.src/Cabin.cc (1)
23-67
: Enhanced CLI Option Definitions in getCli()The restructured command-line option definitions in the
getCli()
function significantly improve clarity and maintainability. Splitting each option into its own multi-line block makes it easier to read and update in the future, with no changes to functionality.🧰 Tools
🪛 Cppcheck (2.10-2)
[performance] 25-25: Function parameter 'str' should be passed by const reference.
(passedByValue)
[performance] 38-38: Function parameter 'other' should be passed by const reference.
(passedByValue)
src/Cmd/Add.cc (1)
25-49
: Enhanced Formatting for Command Options.
The chained method calls (for example in the.setArg()
and various.addOpt()
invocations) have been reformatted into a multi‐line layout. This change improves readability without affecting functionality.src/Cmd/Run.cc (1)
27-38
: Reformatted RUN_CMD Initialization.
The multi-line formatting for theRUN_CMD
constant (including the.setShort()
,.setDesc()
, and.addOpt()
calls) enhances the alignment and readability of the command configuration while preserving the underlying logic.src/Manifest.cc (3)
55-62
: Improved Package Parsing Readability.
Breaking down the retrieval of the package’sedition
andversion
values into multi-line calls clarifies the chain of operations. No functionality is altered.🧰 Tools
🪛 Cppcheck (2.10-2)
[performance] 58-58: Function parameter 'str' should be passed by const reference.
(passedByValue)
168-172
: Clearer Dev Profile Option Level Extraction.
The refactoring of thedevOptLevel
assignment into a multi-line format makes the code easier to follow while maintaining the original behavior.
203-208
: Consistent Formatting for Release Profile Options.
Similar multi-line formatting applied to the extraction ofrelOptLevel
contributes to a uniform style across profile parsing. All changes are cosmetic.src/Cmd/Lint.cc (1)
22-29
: Enhanced Formatting for the--exclude
Option.
Reformatting the definition of the--exclude
option into multiple lines improves its clarity without any changes to its functionality.src/Algos.hpp (1)
59-65
: Reformatted Levenshtein Distance Calculation.
The updated indentation and line breaks in the call tostd::min
(used for computing deletion, insertion, and substitution costs) enhance readability while keeping the algorithm’s logic intact.src/Cmd/Clean.cc (1)
19-24
: Cosmetic Formatting: Enhanced Option Configuration ReadabilityThe multi-line formatting of the
.addOpt()
call—including the detailed breakdown of the option configuration for--profile
—improves the overall readability without affecting functionality.src/Cli.hpp (2)
199-200
: Cosmetic Formatting: Improved Function Declaration LayoutThe reformatting of the declaration for
missingOptArgumentFor
now spans multiple lines. This improves clarity and consistency with other declarations without changing its behavior.
256-257
: Cosmetic Formatting: Clearer Declaration for expandOptsSplitting the declaration of
expandOpts
over two lines enhances readability. No functionality is affected by this change.src/Command.cc (2)
174-178
: Cosmetic Formatting: Clearer Return Statement in waitWithOutput()The modified multi-line formatting for the return statement within
Child::waitWithOutput
clarifies the structure of the returnedCommandOutput
object. This is a cosmetic change that does not affect execution.
271-274
: Cosmetic Formatting: Improved Readability in spawn() MethodReformatting the return statement in the parent branch of
Command::spawn()
—by splitting the creation of theChild
object onto multiple lines—enhances clarity without impacting functionality.src/Cli.cc (3)
111-113
: Cosmetic Formatting: Refined Signature Layout for Opt::formatThe updated multi-line splitting of the parameters in the
Opt::format
signature makes the method easier to read and maintain while preserving its functionality.
660-662
: Cosmetic Formatting: Improved Declaration of formatAllSubcmdsBreaking the parameter list for
Cli::formatAllSubcmds
across multiple lines enhances readability without altering the method’s logic or behavior.
709-713
: Cosmetic Formatting: Clarified Help Message ConstructionThe revised chaining of method calls on the temporary
Subcmd{ "..." }
object—especially the multi-line layout for setting the description—improves the clarity of the help command suggestion. This change is purely cosmetic.
Summary by CodeRabbit
Chores
Style
Note: These internal improvements are cosmetic only and do not change any visible functionality for end-users.