Skip to content

Fix all critical, major, minor, and nit code quality issues - #9

Open
bjinwright wants to merge 1 commit into
masterfrom
claude/code-quality-review-YhHAr
Open

Fix all critical, major, minor, and nit code quality issues#9
bjinwright wants to merge 1 commit into
masterfrom
claude/code-quality-review-YhHAr

Conversation

@bjinwright

Copy link
Copy Markdown
Member

Critical:

  • Replace mutable default validators=[] with None across all init signatures (C1)
  • Replace mutable default errors=dict() with None in render() (C2)
  • Remove self.validators = set(self.validators) to preserve validator order (C3)
  • Fix render_static_assets() to use dict.fromkeys() for ordered deduplication (C4)
  • Rewrite setup.py to drop the removed pip.req.parse_requirements() API (C5)

Major:

  • Replace wildcard 'from valley.mixins import *' with explicit valley.properties imports (M1)
  • Fix CSS class joining from comma-separated to space-separated (M2)
  • Fix copy-paste bug in test_booleanfield using StringField for optional case (M3)
  • Add comprehensive Form tests covering iteration, render, and static assets (M4)
  • Add missing envs dependency to pyproject.toml and requirements.txt (M5)

Minor:

  • Quote placeholder attributes in base.html and textarea.html templates (N1, N2)
  • Make TextAreaField rows/cols configurable via init params (N3)
  • Update CKEditor CDN from v4.6.2 to v4.22.1 (N4)
  • Align setup.py version to 1.3.1 and license to Apache-2.0 (N5, N6)
  • Remove deprecated six (Python 2 compat) from requirements.txt (N7)

Nit:

  • Add type hints to all method signatures in fields.py, form.py (T1)
  • Restore BUILTIN_DOC_ATTRS with type annotation (valley contract) (T2)
  • Rename loop variable i -> package in utils.py (T3)
  • Configure bumpversion to update pyproject.toml version (T4)
  • Update README Python version list from EOL (2.7/3.3/3.4/3.5) to 3.8-3.11 (T5)

All 23 tests pass.

https://claude.ai/code/session_01Jgs6rcgzZdwESjA2TzATNV

Critical:
- Replace mutable default validators=[] with None across all __init__ signatures (C1)
- Replace mutable default errors=dict() with None in render() (C2)
- Remove self.validators = set(self.validators) to preserve validator order (C3)
- Fix render_static_assets() to use dict.fromkeys() for ordered deduplication (C4)
- Rewrite setup.py to drop the removed pip.req.parse_requirements() API (C5)

Major:
- Replace wildcard 'from valley.mixins import *' with explicit valley.properties imports (M1)
- Fix CSS class joining from comma-separated to space-separated (M2)
- Fix copy-paste bug in test_booleanfield using StringField for optional case (M3)
- Add comprehensive Form tests covering iteration, render, and static assets (M4)
- Add missing envs dependency to pyproject.toml and requirements.txt (M5)

Minor:
- Quote placeholder attributes in base.html and textarea.html templates (N1, N2)
- Make TextAreaField rows/cols configurable via __init__ params (N3)
- Update CKEditor CDN from v4.6.2 to v4.22.1 (N4)
- Align setup.py version to 1.3.1 and license to Apache-2.0 (N5, N6)
- Remove deprecated six (Python 2 compat) from requirements.txt (N7)

Nit:
- Add type hints to all method signatures in fields.py, form.py (T1)
- Restore BUILTIN_DOC_ATTRS with type annotation (valley contract) (T2)
- Rename loop variable i -> package in utils.py (T3)
- Configure bumpversion to update pyproject.toml version (T4)
- Update README Python version list from EOL (2.7/3.3/3.4/3.5) to 3.8-3.11 (T5)

All 23 tests pass.

https://claude.ai/code/session_01Jgs6rcgzZdwESjA2TzATNV
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants