-
Notifications
You must be signed in to change notification settings - Fork 102
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
Cpp metrics plugin #638
Cpp metrics plugin #638
Conversation
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.
Made a quick initial review based on the code.
plugins/cpp_metrics/parser/include/cppmetricsparser/cppmetricsparser.h
Outdated
Show resolved
Hide resolved
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.
Looks good with the changes 👏
Incremental parsing should be supported by the new plugin as well.
Possible solutions:
|
for (const auto& pair : _astNodeIdCache) | ||
{ | ||
auto file = _ctx.db->query_one<model::File>( | ||
odb::query<model::File>::id == pair.second); | ||
|
||
auto it = _ctx.fileStatus.find(file->path); | ||
if (it != _ctx.fileStatus.end() && | ||
(it->second == cc::parser::IncrementalStatus::DELETED || | ||
it->second == cc::parser::IncrementalStatus::MODIFIED || | ||
it->second == cc::parser::IncrementalStatus::ACTION_CHANGED)) | ||
{ | ||
LOG(info) << "[cxxmetricsparser] Database cleanup: " << file->path; | ||
|
||
_ctx.db->erase_query<model::CppAstNodeMetrics>(odb::query<model::CppAstNodeMetrics>::astNodeId == pair.first); | ||
_astNodeIdCache.erase(pair.first); | ||
} | ||
} |
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.
In the loop above you are already iterating through all files and purging the ones, which has deleted, modified or action changed status. There is no need for another loop or for the _astNodeIdCache
at all, we could simply purge the AST nodes there as well.
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.
I will accept it like this, so we can merge this, and created #670.
Co-authored-by: Máté Cserép <[email protected]>
This is the basis of the C++ metrics plugin. It contains the usual components of a plugin, including the fundamental db tables to store module-level and AST node-level (function, class) metrics. This plugin is intended to help Software Technology lab students start their projects.