Skip to content

chore: remove unused six dependency - #49

Open
sumanjeet0012 wants to merge 2 commits into
multiformats:masterfrom
sumanjeet0012:fix-issue-45-remove-six
Open

sumanjeet0012 wants to merge 2 commits into
multiformats:masterfrom
sumanjeet0012:fix-issue-45-remove-six

Conversation

@sumanjeet0012

Copy link
Copy Markdown
Contributor

Fixes #45

Description

This pull request removes the six dependency from pyproject.toml.

six was originally included as a Python 2/3 compatibility library. However, since this project enforces a minimum Python version of 3.10 (requires-python = ">=3.10, <4.0"), six is completely unnecessary. Furthermore, an inspection of the codebase confirms that six is not imported or used anywhere within the project's source, tests, or documentation.

Changes Made

  • Removed "six>=1.10.0,<2.0" from the dependencies list in pyproject.toml.

Verification

  • Confirmed absence of six usage via grep across multibase/, tests/, and docs/.
  • Verified that all 266 tests pass successfully (pytest tests/).
  • Verified that all linter and formatting checks pass (make lint).

@acul71 acul71 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review summary (PR #49)

Thanks for the clean dependency cleanup — removing unused six from pyproject.toml correctly addresses #45 given requires-python >= 3.10 and no imports of six in the codebase.

Critical blocker — missing newsfragment

  • Severity: CRITICAL / BLOCKER

  • Issue: No newsfragments/45.<type>.rst file is present.

  • Required: Add a newsfragment using issue 45, e.g. newsfragments/45.removal.rst (or 45.misc.rst / 45.internal.rst), with a short user-facing ReST line ending in a newline. Example:

    Removed unused ``six`` dependency (Python 2/3 compatibility shim no longer needed for Python ≥3.10).
    

PR approval is blocked until this file is added.

Other findings

  • Branch sync: Diverged — 6 behind / 1 ahead of origin/master. Dry-run merge: no conflicts. Optional update recommended for currency.
  • Overlap (informational): pyproject.toml also touched by open PRs #56 and #57; watch merge ordering later.
  • Security: None.
  • Local checks (after venv restore): make lint, make typecheck, make test (266 passed), make docs-ci — all exit 0.
  • CI: All GitHub checks pass.

Overall assessment

  • Quality: Good
  • Merge readiness: Needs fixes (newsfragment) — requesting changes
  • Confidence: High

Full report: maintainer artifact downloads/AI-PR-REVIEWS/49/AI-PR-REVIEW-49-0.md (local).

@acul71

acul71 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Hi @sumanjeet0012 — friendly follow-up.

I requested changes on this PR on 2026-09-07 (mainly the missing towncrier newsfragment, plus the other notes in that review). I don’t see any new commits addressing those items yet.

Are you still willing / able to carry this PR forward? If yes, please push the requested updates when you can and I’ll re-review promptly. If your bandwidth has changed, no worries — just say so and we can close it or leave it for someone else to pick up.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
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.

six dependency is listed but unused

2 participants