Skip to content

Conversation

@DinethShakya23
Copy link
Contributor

@DinethShakya23 DinethShakya23 commented Nov 1, 2025

Description:
Add comprehensive Google-style docstrings to the TransactionRecord class to improve documentation quality, maintainability, and developer understanding.

  • Add detailed module-level docstring describing the overall purpose and functionality of transaction_record.py
  • Add comprehensive Google-style docstring to the TransactionRecord class, documenting all dataclass fields
  • Add descriptive docstring for the __repr__(), _from_proto() explaining its conversion process from protobuf to Python objects & _to_proto() detailing serialization back to protobuf format.
  • Update CHANGELOG.md

Related issue(s):
Fixes #683

Notes for reviewer:
This PR introduces documentation-only changes — no functional code or logic has been modified.

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

exploreriii
exploreriii previously approved these changes Nov 1, 2025
@exploreriii
Copy link
Contributor

Hi @DinethShakya23 approved, please rebase else let me know if I should do it for you :)

@DinethShakya23
Copy link
Contributor Author

Hi @exploreriii ,
I have resolved the conflict. Is everything okay now?

@exploreriii
Copy link
Contributor

Hi @DinethShakya23 when you merge from main you may lose the signatures if not done correctly
We recommend instead rebasing, git rebase main -S
(see rebasing.md)
Your commits are no longer verified and will be unable to merge them
Thanks

@DinethShakya23
Copy link
Contributor Author

@exploreriii Got it. I will try to rebase my branch using git rebase main -S to keep the commits verified.
Thanks for pointing that out!

Signed-off-by: DinethShakya23 <dinethshakya19@gmail.com>
@DinethShakya23 DinethShakya23 force-pushed the docs/transactionrecord-docstrings branch from 3ee18db to 7d70486 Compare November 2, 2025 02:04
@DinethShakya23
Copy link
Contributor Author

Hi @exploreriii ,

I think I have completed the rebase as requested. Rebased the branch onto the latest main using "git rebase -S" to ensure the commits remain GPG-signed & merge conflict in CHANGELOG.md was resolved manually & the commit (7d70486) now includes both the DCO sign-off and a valid GPG signature.

Please let me know if everything looks good from your side.

Copy link
Contributor

@exploreriii exploreriii left a comment

Choose a reason for hiding this comment

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

Hi, please check the workflow results and the commits tab - it tells you they are not verified
looks like you can fix this by adding your gpg key
read signing.md please 👍

Image

@DinethShakya23
Copy link
Contributor Author

Thanks! I have added my public key for that GPG key.
Capture06
Now the commit shows as verified. Is it okay now or do I need to re-sign the commit?

@exploreriii
Copy link
Contributor

Should be good! Let's wait for the final tests

@DinethShakya23
Copy link
Contributor Author

@exploreriii
Just wanted to confirm, did I follow the correct steps to rebase? (after merging from main)

@exploreriii exploreriii merged commit f6901e6 into hiero-ledger:main Nov 2, 2025
11 checks passed
@exploreriii
Copy link
Contributor

That's right, thank you very much!

@DinethShakya23 DinethShakya23 deleted the docs/transactionrecord-docstrings branch November 2, 2025 14:44
@DinethShakya23
Copy link
Contributor Author

@exploreriii Thank you! Appreciate your help and guidance!!!

@exploreriii exploreriii mentioned this pull request Nov 7, 2025
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.

chore: Add comprehensive docstrings to TransactionRecord in transaction_record.py

2 participants