Claude/merge open prs q t5 zn#153
Conversation
…test - Added test_read_lines to verify happy path and line stripping. - Added test_read_lines_file_not_found to cover the OSError catch block in read_lines, asserting it returns None and logs to stderr. - Fixed test_sanitize_filename_without_name_special_urls which had an incorrect assertion for URLs with ports (colons are sanitized to hyphens). - Updated Scripts/test_common.py imports to include io, tempfile, and patch. Co-authored-by: Ven0m0 <82972344+Ven0m0@users.noreply.github.com>
…late Co-authored-by: Ven0m0 <82972344+Ven0m0@users.noreply.github.com>
Co-authored-by: Ven0m0 <82972344+Ven0m0@users.noreply.github.com>
… converting to python package Co-authored-by: Ven0m0 <82972344+Ven0m0@users.noreply.github.com>
…655711732854780604' into claude/merge-open-prs-qT5Zn
Current Aviator status
This PR was merged manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time.
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Pull request overview
This PR appears to normalize formatting/whitespace across config and scripts while shifting Python script/tests toward package-style imports under Scripts.*, and updating the GitHub workflow invocation accordingly.
Changes:
- Normalize whitespace/formatting in multiple shell/config/YAML files.
- Update Python imports in scripts/tests to use
Scripts.*module paths. - Update the
update-listsGitHub Actions workflow to run the updater viapython -m.
Reviewed changes
Copilot reviewed 9 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| mise.toml | Removes trailing whitespace / normalizes env var line formatting. |
| Scripts/userscript.sh | Removes trailing whitespace in ruff conditional. |
| Scripts/update-lists.py | Changes import to Scripts.common for package-style usage. |
| Scripts/test_update_lists.py | Attempts to change dynamic import path; currently contains conflict marker. |
| Scripts/test_move_pure_domains.py | Updates dynamic import target filename to underscore variant. |
| Scripts/test_is_pure_domain_logic.py | Switches imports to Scripts.* and avoids hyphenated module import. |
| Scripts/test_deduplicate.py | Switches to Scripts.deduplicate import (comment now misleading). |
| Scripts/test_common.py | Switches to Scripts.common and adds tests for read_lines. |
| Scripts/move-pure-domains.py | Changes import to Scripts.common. |
| Scripts/hosts-creator.sh | Removes a blank line with stray whitespace. |
| Scripts/deduplicate.py | Changes import to Scripts.common. |
| Scripts/build.sh | Trims trailing whitespace around ` |
| .yamllint.yml | Fixes trailing whitespace after keys. |
| .serena/project.yml | Removes trailing whitespace in comment line. |
| .github/workflows/update-lists.yml | Runs update lists via python -m Scripts.update_lists; trims whitespace. |
| .github/workflows/maintain-lists.yml | Removes stray whitespace line. |
| .github/actionlint.yaml | Adjusts YAML formatting for paths/ignore list (but introduces a likely invalid path key). |
|
|
||
| spec = importlib.util.spec_from_file_location("update_lists", "Scripts/update-lists.py") | ||
| spec = importlib.util.spec_from_file_location("update_lists", "Scripts/update_lists.py") | ||
| >>>>>>> origin/jules/fix-sys-path-boilerplate-6564146397324574189 |
There was a problem hiding this comment.
There is an unresolved merge-conflict marker (>>>>>>> ...) in the test file, which will cause a syntax error. Remove the conflict marker and ensure the chosen spec_from_file_location path matches the actual updater script filename in Scripts/.
| >>>>>>> origin/jules/fix-sys-path-boilerplate-6564146397324574189 |
| run: | | ||
| set -e; mkdir -p lists/mirror | ||
| uv run python3 -OO Scripts/update-lists.py --output-dir lists/mirror --max-concurrent 8 | ||
| uv run python3 -OO -m Scripts.update_lists --output-dir lists/mirror --max-concurrent 8 |
There was a problem hiding this comment.
This workflow now runs -m Scripts.update_lists, which requires an importable module named Scripts.update_lists (i.e., a file Scripts/update_lists.py). In this diff, the updater file being modified is Scripts/update-lists.py (hyphen), which cannot be imported as Scripts.update_lists. Rename the script to Scripts/update_lists.py (and update references) or revert the workflow to executing the script by path.
| uv run python3 -OO -m Scripts.update_lists --output-dir lists/mirror --max-concurrent 8 | |
| uv run python3 -OO Scripts/update-lists.py --output-dir lists/mirror --max-concurrent 8 |
|
|
||
| # Import the module dynamically | ||
| file_path = Path(__file__).parent / "move-pure-domains.py" | ||
| file_path = Path(__file__).parent / "move_pure_domains.py" |
There was a problem hiding this comment.
This test now loads move_pure_domains.py, but the corresponding script shown in this PR is move-pure-domains.py (hyphen). As written, the dynamic import will fail with a missing file error. Align the test with the actual filename, or rename the script to move_pure_domains.py and update other references accordingly.
| file_path = Path(__file__).parent / "move_pure_domains.py" | |
| file_path = Path(__file__).parent / "move-pure-domains.py" |
| from Scripts.common import ADGUARD_INDICATORS | ||
|
|
||
| # Import is_pure_domain from move-pure-domains | ||
| move_pure_domains = importlib.import_module("move-pure-domains") | ||
| is_pure_domain = move_pure_domains.is_pure_domain | ||
| from Scripts.move_pure_domains import is_pure_domain |
There was a problem hiding this comment.
These imports assume Scripts.move_pure_domains exists as an importable module (typically Scripts/move_pure_domains.py). In this PR, the script file is Scripts/move-pure-domains.py (hyphen), which cannot be imported as a Python module with that name. Either rename the script to move_pure_domains.py or switch back to a dynamic import by file path.
| paths: | ||
| . github/workflows/*.yml: | ||
| ignore: | ||
| . github/workflows/*.yml: |
There was a problem hiding this comment.
The path key appears to contain an unintended space (. github/workflows/*.yml:) which likely prevents actionlint from matching workflow files under .github/workflows/*.yml. Replace it with the correct path (no space) so the ignore rules apply.
| . github/workflows/*.yml: | |
| .github/workflows/*.yml: |
|
|
||
|
|
||
| from deduplicate import process_content, is_header, is_valid_rule | ||
| # Add current directory to path to allow importing deduplicate |
There was a problem hiding this comment.
The comment says the test adds the current directory to sys.path, but the code no longer does that and instead imports from Scripts.deduplicate. Update/remove the comment to reflect the current import approach to avoid confusion during maintenance.
| # Add current directory to path to allow importing deduplicate | |
| # Import deduplication helpers from the Scripts.deduplicate module |
Summary
Details
Related Issues
How to Validate
Pre-Merge Checklist