complete refactor #49

2bndy5 · 2024-01-02T06:39:49Z

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 Parse clang-tidy and clang-format stdout from buffered strings #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.
Removed dead code (from previous attempts to generate PR reviews).
Added ability to pass positional/optional CLI arguments. These arguments will be treated as paths and automatically included in the not_ignored list of files. However, the paths are still subject to further filtering with --extensions, --lines-changed-only, and --files-changed-only options.

shenxianpeng · 2024-01-02T06:56:56Z

Great job on the refactoring! It's helpful for others to get involved, including myself.

2bndy5 · 2024-01-02T07:16:02Z

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.

2bndy5 · 2024-01-02T07:23:04Z

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 pygit2 to generate the diffs that would be posted in review comments (as suggestions).

codecov · 2024-01-02T08:48:18Z

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (5f67781) 89.20% compared to head (aad7baf) 99.90%.

Files	Patch %	Lines
cpp_linter/rest_api/github_api.py	99.36%	1 Missing ⚠️

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.
📢 Have feedback on the report? Share it here.

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.

resolves #63 Technically, this fixes an undesired behavior introduced in #49.

* remove duplicated file name in tidy comment resolves #63 Technically, this fixes an undesired behavior introduced in #49. * use MD bold syntax when not using raw HTML syntax

2bndy5 added the enhancement New feature or request label Jan 2, 2024

shenxianpeng approved these changes Jan 2, 2024

View reviewed changes

2bndy5 linked an issue Jan 2, 2024 that may be closed by this pull request

Parse clang-tidy and clang-format stdout from buffered strings #48

Closed

2bndy5 force-pushed the refactor branch from 8bb4d24 to a2db0e2 Compare January 2, 2024 08:37

2bndy5 force-pushed the refactor branch 2 times, most recently from a5c796c to 394b91c Compare January 2, 2024 11:38

2bndy5 marked this pull request as ready for review January 2, 2024 19:50

This comment was marked as off-topic.

Sign in to view

2bndy5 mentioned this pull request Jan 2, 2024

generated PR review from clang-format/clang-tidy suggestions #50

Closed

This comment was marked as spam.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as resolved.

Sign in to view

2bndy5 force-pushed the refactor branch from ceaf4d8 to 266c1b4 Compare January 3, 2024 06:57

This comment was marked as off-topic.

Sign in to view

2bndy5 force-pushed the refactor branch from ecc2872 to 79a0024 Compare January 3, 2024 08:45

2bndy5 added 10 commits January 3, 2024 00:47

prevent real CI behavior in unit testing mock http requests

69b45f2

prevent parsing blank XML strings

82a0c69

and ensure they are decoded to UTF-8

monkeypatch get_diff() for shallow checkouts in CI

59b7f61

bring coverage back up to previously on main

d14548d

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

fix some valid codacity concerns

24d437d

test post_feedback()

ffbc3ed

should nearly complete code coverage

include tests in coverage reports

bb91239

2bndy5 force-pushed the refactor branch from 79a0024 to bb91239 Compare January 3, 2024 08:47

fix problem in "is ignored" logic

aad7baf

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.

2bndy5 force-pushed the refactor branch from f7a161d to aad7baf Compare January 4, 2024 20:53

2bndy5 merged commit ecff34c into main Jan 4, 2024
97 checks passed

2bndy5 deleted the refactor branch January 4, 2024 21:17

2bndy5 mentioned this pull request Jan 6, 2024

Link clang-tidy diagnostic names to clang-tidy docs #52

Merged

2bndy5 mentioned this pull request Feb 10, 2024

duplicated file name in tidy comment #63

Closed

2bndy5 added a commit that referenced this pull request Feb 10, 2024

remove duplicated file name in tidy comment

33332a9

resolves #63 Technically, this fixes an undesired behavior introduced in #49.

2bndy5 mentioned this pull request Feb 10, 2024

remove duplicated file name in tidy comment #64

Merged

2bndy5 mentioned this pull request Feb 24, 2024

re-enable thread-comments on private repos #81

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

complete refactor #49

complete refactor #49

2bndy5 commented Jan 2, 2024 •

edited

shenxianpeng commented Jan 2, 2024

2bndy5 commented Jan 2, 2024

2bndy5 commented Jan 2, 2024

codecov bot commented Jan 2, 2024 •

edited

This comment was marked as off-topic.

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 resolved.

This comment was marked as off-topic.

complete refactor #49

complete refactor #49

Conversation

2bndy5 commented Jan 2, 2024 • edited

shenxianpeng commented Jan 2, 2024

2bndy5 commented Jan 2, 2024

2bndy5 commented Jan 2, 2024

codecov bot commented Jan 2, 2024 • edited

Codecov Report

This comment was marked as off-topic.

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 resolved.

This comment was marked as off-topic.

2bndy5 commented Jan 2, 2024 •

edited

codecov bot commented Jan 2, 2024 •

edited