Repository navigation
feat: expose code property on Encoder class - #53
sumanjeet0012 wants to merge 2 commits into
Conversation
acul71
left a comment
There was a problem hiding this comment.
Request changes — missing newsfragment (blocker)
Thanks for this clean, focused fix for #47. The Encoder.code property matches the issue proposal and looks correct; CI and local make lint / typecheck / test / docs-ci all pass.
Blocker
Please add a towncrier newsfragment:
- Path:
newsfragments/47.feature.rst - Content (example):
Added a public ``Encoder.code`` property exposing the multibase prefix byte. - Must end with a trailing newline
Newsfragments are mandatory for approval on this repo.
Also recommended (non-blocking)
- Rebase onto current
origin/master(branch is ~6 commits behind; dry-run merge is clean today). - Note overlap with open PR #54 on the same files — coordinate merge order.
- Optional: assert another encoding for
.code(e.g.base16→b"f") and/or mentionencoder.codein the README Encoder example.
Happy to re-review once 47.feature.rst is added.
|
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>
acul71
left a comment
There was a problem hiding this comment.
Re-review — newsfragment OK; please sync with master
Thanks for adding newsfragments/47.feature.rst — the prior blocker is resolved.
Still needed before approval
Please merge or rebase onto current origin/master. Remote tip 8dc5671 is 10 behind / 2 ahead. I merged master into a local worktree with no conflicts, and all checks passed there, but maintainerCanModify is false so I cannot push the sync for you.
Also helpful: enable “Allow edits from maintainers” on the PR so we can sync next time if needed.
Validation (local worktree after merging master)
Encoder.codelooks correct; test coverage still present.- Local:
make lint,typecheck,test(358 passed, 9 skipped),docs-ci— all green. - GitHub CI on the current tip: green.
Happy to approve as soon as the branch is synced with master.
Fixes #47
Description
This pull request adds a public
.codeproperty to theEncoderclass.Changes
self._codec.codevia a read-only propertycode.py-multibaseinto alignment withgo-multibase, which exposes the encoding code viaEncoder.Encoding().encoder.codereturns the correct multibase prefix byte.