-
-
Notifications
You must be signed in to change notification settings - Fork 7
Enable parallelism #92
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
See cpp-linter/cpp-linter#92 for the related CLI updates.
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.
This was a pleasant surprise! Thanks for taking this on.
🎉 Benchmarking at repo root on my intel i7-9700 CPU
|
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'm also noticing that you don't use a word delimiter _
in your variable names (which should be snake-cased according to PEP8). This is throwing my spellchecker in a frenzy.
If you want to change some names, please locate the names so I could update them. |
Thanks for the suggestions! I don't use a spell checker, so I just wanted to know the spell checker output of your environment. |
I'm adding a pre-commit hook to run codespell in #93. I realized my request wasn't integrated into our CI checks. ruff is pretty relaxed about variable naming in a function's scope. The actual spell checker that I'm using is a VSCode extension (not the codespell package). |
Co-authored-by: Brendan <2bndy5@gmail.com>
I just finished variable renaming! No more force pushes. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #92 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 20 20
Lines 1392 1437 +45
=========================================
+ Hits 1392 1437 +45 ☔ View full report in Codecov by Sentry. |
I think there are some suspicious missing lines in the coverage report. I will investigate them first. |
The "missing" lines are actually covered in tests (checked manually), but coverage could not track the changes. Are we ok to skip them or need some workarounds? |
Co-authored-by: Brendan <2bndy5@gmail.com>
Well, I'm still puzzled why only the I'd rather not skip the lines just to make coverage report "look" better. I prefer we actually have reproducible results to measure for code coverage. |
I'll experiment for other methods to get coverage report correctly and share if any succeded. I think this looks like an upstream bug tho. |
I'm seeing a lot of .coverage files created in the tests folder. When I run |
If I move all the rogue .coverage files into repo root and run This skipping "duplicate data" isn't a new thing, but spawning the coverage files in the tests folder is a new thing. I hesitate to call it a problem because I don't know the inner-workings of the coverage package. None of this actually changed the overall code coverage reported. |
Yes, I also just found out that the monkeypatch might cause problems for subprocesses. Fixed in 696d83c. |
The rogue .coverage file
|
Oh, ok. It seems the tests are not very pleased with out-of-order logs. I'll update them shortly. |
I must say, you are picking up on the cause of these errors faster than I'm accustomed to. Its very refreshing! |
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.
Oh, ok. It seems the tests are not very pleased with out-of-order logs
That was it completely! coverage is restored and new functionality verified 💯
I should thank you and the maintainers first for creating such a great package/workflow. Great work! |
See cpp-linter/cpp-linter#92 for the related CLI updates.
See cpp-linter/cpp-linter#92 for the related CLI updates.
* feat: add --jobs parameter to action See cpp-linter/cpp-linter#92 for the related CLI updates. * adjustments for docs --------- Co-authored-by: Brendan <2bndy5@gmail.com>
Closes #74. I also reviewed #73.