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.

Sign up for GitHub

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

Jump to bottom

Implement cyclomatic complexity (McCabe) metric on a function level #689

Merged
merged 8 commits into from
Jan 16, 2024
Merged

Implement cyclomatic complexity (McCabe) metric on a function level #689

merged 8 commits into from
Jan 16, 2024

Conversation

mozesl
Copy link
Collaborator

@mozesl mozesl commented Dec 3, 2023

Closes #682

@mozesl mozesl requested a review from mcserep December 3, 2023 15:16
@mozesl mozesl added Kind: Enhancement 🌟 Plugin: C++ Issues related to the parsing and presentation of C++ projects. Plugin: Metrics Issues related to the code metrics plugin. labels Dec 3, 2023
@intjftw intjftw self-requested a review December 3, 2023 16:09
intjftw
intjftw requested changes Dec 12, 2023
View reviewed changes
Copy link
Collaborator

@intjftw intjftw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the plugin, here are my questions and change requests.
I haven't checked how the tests work yet, when I do, I'll complement my review.

plugins/cpp/parser/src/clangastvisitor.h Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not particularly satisfied with this solution as it creates strong coupling between the cpp and the cpp_metrics plugin. However, I couldn't come up with a better solution either so be it.

plugins/cpp/model/include/model/cppfunction.h Outdated Show resolved Hide resolved
plugins/cpp/model/include/model/cppfunction.h Outdated Show resolved Hide resolved
plugins/cpp_metrics/parser/src/cppmetricsparser.cpp Outdated Show resolved Hide resolved
plugins/cpp/parser/src/clangastvisitor.h Outdated Show resolved Hide resolved
plugins/cpp_metrics/test/CMakeLists.txt Outdated Show resolved Hide resolved
plugins/cpp_metrics/test/src/cppmetricstest.cpp Outdated Show resolved Hide resolved
plugins/cpp_metrics/test/sources/service/CMakeLists.txt Outdated Show resolved Hide resolved
plugins/cpp_metrics/test/sources/parser/mccabe.cpp Outdated Show resolved Hide resolved
intjftw
intjftw approved these changes Jan 15, 2024
View reviewed changes
Copy link
Collaborator

@intjftw intjftw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried running the tests now, too. They pass and seem to satisfy the requirements. The modifications also work well, so I approve this PR.

@intjftw intjftw merged commit 080a472 into Ericsson:master Jan 16, 2024
7 checks passed
@mcserep mcserep mentioned this pull request Jan 16, 2024
Merged
@intjftw intjftw mentioned this pull request Feb 2, 2024
Merged
@mcserep
Copy link
Collaborator

mcserep commented Feb 3, 2024

@intjftw Next time please squash the commits according to the project guidelines, as the PR did not really have a clean version history (multiple "debug", "fix" and similar commits).

@mcserep mcserep mentioned this pull request Mar 3, 2024
Closed
@mcserep mcserep mentioned this pull request Mar 14, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@intjftw intjftw intjftw approved these changes

@mcserep mcserep Awaiting requested review from mcserep

Assignees

@mozesl mozesl

Labels
Kind: Enhancement 🌟 Plugin: C++ Issues related to the parsing and presentation of C++ projects. Plugin: Metrics Issues related to the code metrics plugin.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cyclomatic Complexity at Function Level for C++
3 participants
@mozesl @mcserep @intjftw