-
Notifications
You must be signed in to change notification settings - Fork 70
[Good First Issue]: fix removeSignature docstring to match file conventions #1226
Description
🆕🐥 First-Time Friendly
This issue is especially welcoming for people who are new to contributing to the Hiero C++ SDK.
We know that opening your first pull request can feel like a big step.
Issues labeled Good First Issue are designed to make that experience easier, clearer, and more comfortable.
No prior knowledge of Hiero, Hedera, or distributed ledger technology is required.
Just a basic familiarity with C++ and Git is more than enough to get started.
👾 Description of the Issue
The removeSignature method in Transaction.h has a docstring that is inconsistent with the style used throughout the rest of the file.
The current docstring reads:
/**
* This method removes all signatures from the transaction based on the public key provided.
*
* @param publicKey The public key associated with the signature to remove.
* @return The removed signatures.
* @throws IllegalStateException if transaction is not frozen or the given key didn't sign it off.
*/There are three issues:
- Voice: Every other docstring in the file uses imperative voice (e.g., "Freeze this Transaction", "Add a signature to this Transaction"). This one uses "This method removes..." instead.
- Clarity: The phrase "removes all signatures from the transaction based on the public key provided" reads too similarly to the neighboring
removeAllSignaturesmethod, which could confuse readers. - Formatting: The
@throwsline uses a lowercase "if" and awkward phrasing ("didn't sign it off"), while the rest of the file uses capitalized "If" and clearer wording.
For comparison, the sibling removeAllSignatures docstring directly below it already follows the correct style:
/**
* Remove all signatures from the transaction.
*
* @return The removed signatures grouped by their associated public key.
* @throws IllegalStateException If the transaction is not frozen.
*/💡 Proposed Solution
Replace the removeSignature docstring with a version that matches the file's existing style conventions:
- Use imperative voice ("Remove..." not "This method removes...")
- Clearly distinguish this method from
removeAllSignatures - Capitalize "If" in
@throwsand use natural phrasing
👩💻 Implementation Steps
- Open
src/sdk/main/include/Transaction.h - Locate the
removeSignaturemethod declaration (directly below thegetSignaturesmethod) - Replace the existing docstring:
/**
* This method removes all signatures from the transaction based on the public key provided.
*
* @param publicKey The public key associated with the signature to remove.
* @return The removed signatures.
* @throws IllegalStateException if transaction is not frozen or the given key didn't sign it off.
*/with:
/**
* Remove the signature(s) associated with a specific public key from the transaction.
*
* @param publicKey The public key whose signature should be removed.
* @return The removed signatures.
* @throws IllegalStateException If the transaction is not frozen or the public key has not signed this transaction.
*/- Verify no other code is changed — this is a comment-only update.
✅ Acceptance Criteria
To merge a pull request for this issue:
- Scope: Changes are limited to the
removeSignaturedocstring inTransaction.h - Behavior: No SDK behavior or public API changes
- Tests: Existing CI checks pass
- Review: All code review feedback addressed
📋 Step-by-Step Contribution Guide
- Comment
/assignto request the issue - Wait for assignment
- Fork the repository and create a branch
- Set up the project by following the instructions in
README.md - Make the requested changes
- Sign each commit using
-s -S - Push your branch and open a pull request
Read Workflow Guide for step-by-step workflow guidance.
Read README.md for setup instructions.
❗ Pull requests cannot be merged without S and s signed commits.
See the Signing Guide.
🤔 Additional Information
This issue was identified during the review of #1218 (which added the removeSignature and removeAllSignatures methods). The sibling removeAllSignatures docstring was fixed in that PR, but the removeSignature docstring was not caught until after merge.
File to Change
| File | Change |
|---|---|
src/sdk/main/include/Transaction.h |
Update removeSignature docstring |
If you have questions while working on this issue, feel free to ask!
You can reach the community and maintainers here:
Hiero-SDK-C++ Discord
Maintainers are happy to help first-time contributors succeed!