Conversation
See cpp-linter/cpp-linter#92 for the related CLI updates.
2bndy5
left a comment
There was a problem hiding this comment.
This was a pleasant surprise! Thanks for taking this on.
🎉 Benchmarking at repo root on my intel i7-9700 CPU |
2bndy5
left a comment
There was a problem hiding this comment.
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 <[email protected]>
|
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 <[email protected]>
|
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! |
2bndy5
left a comment
There was a problem hiding this comment.
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 <[email protected]>
Closes #74. I also reviewed #73.