Fix acknowledge when run on multiple files #244
Conversation
|
Thank you for contributing! 👋 |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes issue #236 where the acknowledge-existing-violations command failed when processing multiple files. The bug occurred because lint re-runs during error acknowledgment were not filtered to the specific file being processed, causing line number and issue mismatches.
Key changes:
- Modified
get_errors_to_processcalls to use[bad_file]instead offile_or_dirto scope lint operations to the current file - Added comprehensive test coverage for multiple file scenarios with both normal and aggressive modes
- Updated pytest and linter configurations to exclude new test input files
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ni_python_styleguide/_acknowledge_existing_errors/init.py | Fixed lint re-runs to filter to the specific file being processed (lines 92, 131) |
| tests/test_cli/test_acknowledge_existing_errors.py | Added two new test functions to verify multiple file handling works correctly |
| tests/test_cli/acknowledge_existing_errors_multiple_files/input/acknowledge_function_signatures.py | New test input file with various linting violations for testing |
| tests/test_cli/acknowledge_existing_errors_multiple_files/input/acknowledge_doc_lines.py | New test input file with docstring and line length violations |
| tests/test_cli/acknowledge_existing_errors_multiple_files/input/acknowledge_blank_file.py | New empty test input file |
| pyproject.toml | Updated test and linter exclusions to ignore new test input files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pass | ||
|
|
||
| def add(self, o): | ||
| """Provide an examples of doc strings that are too long. |
There was a problem hiding this comment.
Grammar error: "an examples" should be "an example"
| """Provide an examples of doc strings that are too long. | |
| """Provide an example of doc strings that are too long. |
| pass | ||
|
|
||
| def add(self, o): | ||
| """Provide an examples of doc strings that are too long. |
There was a problem hiding this comment.
Grammar error: "an examples" should be "an example"
|
|
||
|
|
||
| def method2(): | ||
| """Provide an examples of doc strings that are too long. |
There was a problem hiding this comment.
Grammar error: "an examples" should be "an example"
There was a problem hiding this comment.
@copilot open a new pull request to apply changes switching all new instances of "Provide an examples" to "Provide an example"
2 participants
Should fix #236
Turns out that on handling emergent issues (i.e., on a second round), the previous code attempted to process the line number and issue for all files but only in the current file (because re-running the lint was not filtered to the file under work).