-
-
Notifications
You must be signed in to change notification settings - Fork 8
complete refactor #49
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
Conversation
Great job on the refactoring! It's helpful for others to get involved, including myself. |
It was messy to begin with, even back when it was just a complicated bash script. And I got tired of patching up other patches of other patches... This was almost a complete rewrite. Most of these improvements have come from learning Dart and Rust languages. I'm not too fond of using Dart for this project, but Rust is a very feasible solution (see cpp-linter/cpp_linter_rs#3). The changes here will bring this python code to near identical structure with my Rust port. That way I can properly compare benchmark performances between the 2 implementations. |
I have a new idea in mind that involves a generated PR review for either clang-format suggestions or clang-tidy suggestions (possibly even both in a unified review 🤞🏼 ). It would use more of |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #49 +/- ##
===========================================
+ Coverage 89.20% 99.90% +10.70%
===========================================
Files 9 18 +9
Lines 769 1091 +322
===========================================
+ Hits 686 1090 +404
+ Misses 83 1 -82 ☔ View full report in Codecov by Sentry. |
a5c796c
to
394b91c
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as off-topic.
This comment was marked as off-topic.
This uses a much more object-oriented approach. - Functions are properly organized in modules with appropriate names. - REST API usage is isolated in class derived from an abstract base class that will easily allow integrating other git servers' REST API. - clang-format output is filtered by lines (when applicable) as the XML is parsed. - The comment is made after all clang tool output is parsed. - All clang tool output is parsed from buffered strings (addresses #48). - There isn't any cached files when verbosity is not set to debug mode. - Tests can now use mocked HTTP requests (no more fear of hitting REST API rate limits). - Updated docs to reflect the new package structure.
and ensure they are decoded to UTF-8
only check format comment if checks failed not all versions of clang-format trigger on our test case for using a .clang-format config file
should nearly complete code coverage
- merge run module into root __init__ module - did some doc review - CLI supports file paths/names given as positional args - added a test for this because the values are used as the starting value for the `not_ignored` list. - Change acceptable verbosity input values to be "debug" or "10" - Updated help strings in CLI for narrow terminals
This reintroduces the approach where we filter the list of changed files according to diff information about lines-changed-only value. The advantage (which still holds true) is that we don't run a clang tool on a file if there is no changes of concern.
Due to python's lazy logic evaluation, we need to check if a path is first explicitly not-ignored before checking if said source is ignored. Also modified test to check sub directories and use files (not folders) in expected results.
This uses a much more object-oriented approach.
not_ignored
list of files. However, the paths are still subject to further filtering with--extensions
,--lines-changed-only
, and--files-changed-only
options.