Skip to content

fix(scripts): add explicit utf-8 encoding to read_text/write_text calls (fixes #36649) - #36828

Closed
Morad37 wants to merge 1 commit into
NousResearch:mainfrom
Morad37:fix/36649-plw1514-utf8-encoding
Closed

fix(scripts): add explicit utf-8 encoding to read_text/write_text calls (fixes #36649)#36828
Morad37 wants to merge 1 commit into
NousResearch:mainfrom
Morad37:fix/36649-plw1514-utf8-encoding

Conversation

@Morad37

@Morad37 Morad37 commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Adds encoding=utf-8 to 5 read_text/write_text calls in:

  • scripts/lint_diff.py (2 calls)
  • scripts/profile-tui.py (1 call)
  • scripts/run_tests_parallel.py (2 calls)

The project enables Ruff rule PLW1514 globally, but these calls were missing the encoding parameter. All other calls in these files already had explicit encoding.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have labels Jun 1, 2026

@mxnstrexgl mxnstrexgl left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated PR Review

Security Scan

  • ✓ No hardcoded secrets, injection sinks, unsafe deserialization, or dependency red flags found by this automated scan.

Code Quality

  • ✓ No blocking code-quality issues found by this automated scan.
  • ℹ️ No test file changes detected; verify existing coverage exercises this behavior.

Summary

Status: APPROVE — security findings: 0, quality suggestions: 0.

Automated review; raw diff content intentionally omitted.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused fix. The premise remains valid on current main: the five targeted calls are still bare at scripts/lint_diff.py:33,200, scripts/profile-tui.py:519, and scripts/run_tests_parallel.py:482,504. The proposed encoding="utf-8" arguments match the repository's PLW1514 policy in pyproject.toml:375-386 and the existing nearby call style.

Current main has moved since the PR base and GitHub reports the branch as conflict-dirty, so this should be salvaged by reapplying the same five substitutions to HEAD rather than merging the stale branch directly.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing as resolved by PR #71078 (merged, commit d372fda), which retired the bare read_text/write_text class codebase-wide: 139 sites guarded (11 contributor PRs salvaged with authorship preserved — earliest submitters credited — plus an AST sweep), with a CI linter rule and AST guard test preventing regressions. Every site this PR touches is guarded on current main — verified per-site before closing. Thanks for the fix; the number of independent PRs on this class is what escalated it to the campaign.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants