-
Notifications
You must be signed in to change notification settings - Fork 142
refactor: create account transaction without alias #1334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: create account transaction without alias #1334
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1334 +/- ##
=======================================
Coverage 92.29% 92.29%
=======================================
Files 139 139
Lines 8515 8515
=======================================
Hits 7859 7859
Misses 656 656 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the account_create_transaction_without_alias.py example file to improve code modularity by breaking down the monolithic create_account_without_alias() function into smaller, focused functions.
Key changes:
- Extracted key generation into
generate_account_key() - Split account creation logic into
create_account_without_alias()that returns the account ID - Separated account info retrieval into
fetch_account_info() - Isolated output printing into
print_account_summary() - Moved error handling to the
main()function
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
examples/account/account_create_transaction_without_alias.py |
Refactored single large function into four smaller, focused functions for better separation of concerns |
CHANGELOG.md |
Added entry documenting the refactoring change |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRefactored Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Pre-merge checks✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.mdexamples/account/account_create_transaction_without_alias.py
🧰 Additional context used
📓 Path-based instructions (1)
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.Priority 1 - Correctness:
- Verify transaction lifecycle chain (construction -> freeze_with -> sign -> execute).
- Ensure
freeze_with(client)is called BEFORE signing.- Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase.- Ensure response validation checks
receipt.statusagainstResponseCodeenums (e.g.,ResponseCode.SUCCESS).Priority 2 - Transaction Lifecycle:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit
.execute(client)calls.- Verify response property extraction (e.g., using
.token_id,.account_id,.serial_numbers).- Ensure error handling uses
ResponseCode(receipt.status).namefor clarity.Priority 3 - Naming & Clarity:
- Enforce role-based naming:
operator_id/_key,treasury_account_id/_key,receiver_id/_key.- Use
_idsuffix for AccountId and_keysuffix for PrivateKey variables.- Validate negative examples explicitly check for failure codes (e.g.,
TOKEN_HAS_NO_PAUSE_KEY).- Ensure logical top-to-bottom flow without ambiguity.
Priority 4 - Consistency:
- Verify standard patterns:
def main(),if __name__ == "__main__":,load_dotenv().- IMPORT RULES:
- Accept both top-level imports (e.g.,
from hiero_sdk_python import PrivateKey) and fully qualified imports (e.g.,from hiero_sdk_python.crypto.private_key import PrivateKey).- STRICTLY validate that the import path actually exists in the project structure. Compare against other files in
/examplesor your knowledge of the SDK file tree.- Flag hallucinations immediately (e.g.,
hiero_sdk_python.keysdoes not exist).- Check for
try-exceptblocks withsys.exit(1)for critical failures.Priority 5 - User Experience:
- Ensure comments explain SDK usage patterns (for users,...
Files:
examples/account/account_create_transaction_without_alias.py
🧬 Code graph analysis (1)
examples/account/account_create_transaction_without_alias.py (2)
src/hiero_sdk_python/crypto/private_key.py (2)
PrivateKey(14-471)public_key(305-309)src/hiero_sdk_python/account/account_create_transaction.py (2)
AccountCreateTransaction(27-380)set_key_without_alias(94-107)
🪛 Ruff (0.14.10)
examples/account/account_create_transaction_without_alias.py
80-82: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Agent
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build-and-test (3.10)
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.12)
- GitHub Check: build-and-test (3.11)
- GitHub Check: run-examples
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (2)
CHANGELOG.md (1)
10-10: LGTM!The changelog entry accurately reflects the refactoring work and follows the project's conventions.
examples/account/account_create_transaction_without_alias.py (1)
106-116: LGTM with upstream fixes!The main flow correctly orchestrates the modular functions. Good use of try-except with sys.exit(1) for critical failures.
Note: This will work correctly once the type hints and parameter changes suggested in previous comments are applied.
|
Hi, this is MergeConflictBot. Please resolve these conflicts locally and push the changes. To assist you, please read: Thank you for contributing! |
aceppaluni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tech0priyanshu This is looking fantastic 👍
Please move your changelog entry when possible, thank you!!
82ee2a8 to
47cadeb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/account/account_create_transaction_without_alias.py (1)
14-32: Fix import issues: addPublicKeyand use built-intuple.Two import-related issues:
PublicKeyis used in type hints (lines 46, 54) but not imported, causing undefined name errorstyping.Tupleis deprecated in Python 3.9+; use the built-intupleinstead🔎 Proposed fix
-from typing import Tuple import os import sys import json from dotenv import load_dotenv from examples.utils import info_to_dict from hiero_sdk_python import ( Client, PrivateKey, + PublicKey, AccountCreateTransaction, AccountInfoQuery, Network, AccountId, AccountInfo, Hbar, ResponseCode, )Then update the return type annotation:
-def generate_account_key() -> Tuple[PrivateKey, 'PublicKey']: +def generate_account_key() -> tuple[PrivateKey, PublicKey]:And the parameter type:
-def create_account_without_alias(client: Client, account_public_key: 'PublicKey', account_private_key: PrivateKey) -> AccountId: +def create_account_without_alias(client: Client, account_public_key: PublicKey, account_private_key: PrivateKey) -> AccountId:
♻️ Duplicate comments (1)
CHANGELOG.md (1)
11-11: Remove duplicate changelog entry from "Added" section.Refactoring existing code is a change, not an addition. This entry is correctly placed under "Changed" (line 15) and should be removed from the "Added" section to avoid duplication.
🔎 Proposed fix
### Added - Beginner issue documentation and updated GFI and GFIC templates and documentation -- Refactored `account_create_transaction_without_alias.py` into smaller, modular functions. - Enable auto assignment to good first issues (#1312), archived good first issue support team notification. Changed templates with new assign instruction.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.mdexamples/account/account_create_transaction_without_alias.py
🧰 Additional context used
📓 Path-based instructions (1)
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.Priority 1 - Correctness:
- Verify transaction lifecycle chain (construction -> freeze_with -> sign -> execute).
- Ensure
freeze_with(client)is called BEFORE signing.- Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase.- Ensure response validation checks
receipt.statusagainstResponseCodeenums (e.g.,ResponseCode.SUCCESS).Priority 2 - Transaction Lifecycle:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit
.execute(client)calls.- Verify response property extraction (e.g., using
.token_id,.account_id,.serial_numbers).- Ensure error handling uses
ResponseCode(receipt.status).namefor clarity.Priority 3 - Naming & Clarity:
- Enforce role-based naming:
operator_id/_key,treasury_account_id/_key,receiver_id/_key.- Use
_idsuffix for AccountId and_keysuffix for PrivateKey variables.- Validate negative examples explicitly check for failure codes (e.g.,
TOKEN_HAS_NO_PAUSE_KEY).- Ensure logical top-to-bottom flow without ambiguity.
Priority 4 - Consistency:
- Verify standard patterns:
def main(),if __name__ == "__main__":,load_dotenv().- IMPORT RULES:
- Accept both top-level imports (e.g.,
from hiero_sdk_python import PrivateKey) and fully qualified imports (e.g.,from hiero_sdk_python.crypto.private_key import PrivateKey).- STRICTLY validate that the import path actually exists in the project structure. Compare against other files in
/examplesor your knowledge of the SDK file tree.- Flag hallucinations immediately (e.g.,
hiero_sdk_python.keysdoes not exist).- Check for
try-exceptblocks withsys.exit(1)for critical failures.Priority 5 - User Experience:
- Ensure comments explain SDK usage patterns (for users,...
Files:
examples/account/account_create_transaction_without_alias.py
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
14-14: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🪛 Ruff (0.14.10)
examples/account/account_create_transaction_without_alias.py
14-14: typing.Tuple is deprecated, use tuple instead
(UP035)
46-46: Undefined name PublicKey
(F821)
54-54: Undefined name PublicKey
(F821)
74-76: Avoid specifying long messages outside the exception class
(TRY003)
81-83: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build-and-test (3.11)
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.12)
- GitHub Check: build-and-test (3.10)
- GitHub Check: run-examples
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (6)
examples/account/account_create_transaction_without_alias.py (6)
38-44: LGTM! Modern client initialization pattern.The use of
Client.from_env()follows the simplified SDK pattern and properly returns the client for downstream use.
46-52: LGTM! Correctly returns both keys.This addresses the previous review feedback about returning both private and public keys instead of just the private key. The function now properly exposes both for explicit use in account creation.
54-86: Excellent! All critical transaction lifecycle requirements met.This implementation correctly follows the SDK example guidelines:
- ✅ Proper transaction lifecycle: construction →
freeze_with(client)→sign()→execute()- ✅ Receipt status validation with
ResponseCode(lines 73-76)- ✅ Uses public key in
set_key_without_alias()(line 63) for clarity- ✅ Validates
account_idis not None (lines 80-83)This addresses all previous critical review feedback.
Optional style refinement: Lines 74-76 and 81-83 trigger Ruff's TRY003 (long messages outside exception class), but this is acceptable for educational examples where inline messages improve readability.
88-95: LGTM! Clean query pattern with proper type hints.The function correctly demonstrates the query pattern and includes type annotations as requested in previous reviews.
97-105: LGTM! Proper type hints and clear presentation logic.Type annotations are complete, and the function clearly separates presentation concerns.
107-117: LGTM! Clear orchestration with appropriate error handling.The single try-except block provides clean error handling for the example. While a past review suggested granular error handling for each step, the current approach is simpler and more readable for educational examples—the exception traceback will still identify the specific failure point.
|
@manishdait @aceppaluni @exploreriii @Adityarya11 Thanks for your review. I’ve done my best to put everything in place. Please have a look. |
41075c0 to
76e00fe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/account/account_create_transaction_without_alias.py (1)
14-32: Import PublicKey and use modern type hint syntax.The code uses
'PublicKey'in type hints (lines 46, 54) butPublicKeyis not imported, causing aNameError. Additionally,typing.Tupleis deprecated in Python 3.10+ in favor of the built-intuple.🔎 Proposed fix
-from typing import Tuple import os import sys import json from dotenv import load_dotenv from examples.utils import info_to_dict from hiero_sdk_python import ( Client, PrivateKey, + PublicKey, AccountCreateTransaction, AccountInfoQuery, Network, AccountId, AccountInfo, Hbar, ResponseCode, )Then update the type hints to use modern syntax:
-def generate_account_key() -> Tuple[PrivateKey, 'PublicKey']: +def generate_account_key() -> tuple[PrivateKey, PublicKey]:-def create_account_without_alias(client: Client, account_public_key: 'PublicKey', account_private_key: PrivateKey) -> AccountId: +def create_account_without_alias(client: Client, account_public_key: PublicKey, account_private_key: PrivateKey) -> AccountId:Based on coding guidelines Priority 4: "STRICTLY validate that the import path actually exists" and Priority 1: "Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase."
♻️ Duplicate comments (2)
CHANGELOG.md (1)
10-10: Recategorize refactoring under "Changed" section.This entry describes refactoring existing code into modular functions. Per changelog conventions, refactoring should be listed under "Changed" rather than "Added" since it modifies existing functionality without adding new features.
🔎 Proposed fix
Move the entry from "Added" to the "Changed" section below (after line 15):
### Added -- Refactored `account_create_transaction_without_alias.py` into smaller, modular functions. - Beginner issue documentation and updated GFI and GFIC templates and documentation ### Changed +- Refactored `account_create_transaction_without_alias.py` into smaller, modular functions. - Added unit test for 'endpoint.py' to increase coverage.examples/account/account_create_transaction_without_alias.py (1)
97-105: Consider using AccountInfo's built-in string representation.The current implementation uses
info_to_dict()utility which works correctly. However,AccountInfohas a built-in__str__()method that could simplify the code:🔎 Alternative approach
def print_account_summary(account_info: AccountInfo) -> None: """Print account summary information.""" - out = info_to_dict(account_info) print("Account Info:") - print(json.dumps(out, indent=2) + "\n") + print(account_info) print( "✅ contract_account_id (no alias, zero-padded): " f"{account_info.contract_account_id}" )Note: If you use this approach, you can also remove the
jsonimport and theinfo_to_dictimport if they're no longer needed.Based on learnings, a past reviewer suggested this simplification.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.mdexamples/account/account_create_transaction_without_alias.py
🧰 Additional context used
📓 Path-based instructions (1)
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.Priority 1 - Correctness:
- Verify transaction lifecycle chain (construction -> freeze_with -> sign -> execute).
- Ensure
freeze_with(client)is called BEFORE signing.- Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase.- Ensure response validation checks
receipt.statusagainstResponseCodeenums (e.g.,ResponseCode.SUCCESS).Priority 2 - Transaction Lifecycle:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit
.execute(client)calls.- Verify response property extraction (e.g., using
.token_id,.account_id,.serial_numbers).- Ensure error handling uses
ResponseCode(receipt.status).namefor clarity.Priority 3 - Naming & Clarity:
- Enforce role-based naming:
operator_id/_key,treasury_account_id/_key,receiver_id/_key.- Use
_idsuffix for AccountId and_keysuffix for PrivateKey variables.- Validate negative examples explicitly check for failure codes (e.g.,
TOKEN_HAS_NO_PAUSE_KEY).- Ensure logical top-to-bottom flow without ambiguity.
Priority 4 - Consistency:
- Verify standard patterns:
def main(),if __name__ == "__main__":,load_dotenv().- IMPORT RULES:
- Accept both top-level imports (e.g.,
from hiero_sdk_python import PrivateKey) and fully qualified imports (e.g.,from hiero_sdk_python.crypto.private_key import PrivateKey).- STRICTLY validate that the import path actually exists in the project structure. Compare against other files in
/examplesor your knowledge of the SDK file tree.- Flag hallucinations immediately (e.g.,
hiero_sdk_python.keysdoes not exist).- Check for
try-exceptblocks withsys.exit(1)for critical failures.Priority 5 - User Experience:
- Ensure comments explain SDK usage patterns (for users,...
Files:
examples/account/account_create_transaction_without_alias.py
🧬 Code graph analysis (1)
examples/account/account_create_transaction_without_alias.py (2)
src/hiero_sdk_python/client/client.py (1)
Client(28-255)src/hiero_sdk_python/account/account_create_transaction.py (1)
set_key_without_alias(94-107)
🪛 LanguageTool
CHANGELOG.md
[grammar] ~11-~11: Ensure spelling is correct
Context: ... and updated GFI and GFIC templates and documentation - Refactored `account_create_transaction_w...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
15-15: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🪛 Ruff (0.14.10)
examples/account/account_create_transaction_without_alias.py
14-14: typing.Tuple is deprecated, use tuple instead
(UP035)
46-46: Undefined name PublicKey
(F821)
54-54: Undefined name PublicKey
(F821)
74-76: Avoid specifying long messages outside the exception class
(TRY003)
81-83: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build-and-test (3.11)
- GitHub Check: build-and-test (3.10)
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.12)
- GitHub Check: run-examples
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (4)
examples/account/account_create_transaction_without_alias.py (4)
46-52: LGTM! Key generation logic is correct.The function properly generates both private and public keys and returns them as a tuple, addressing previous reviewer feedback about returning both keys for explicit usage downstream.
54-86: Transaction lifecycle implementation is correct.The function properly implements the required transaction lifecycle:
- ✅ Constructs transaction with
set_key_without_alias(account_public_key)- ✅ Calls
freeze_with(client)before signing- ✅ Signs with the account's private key
- ✅ Executes and validates
ResponseCode.SUCCESS- ✅ Verifies
account_idpresence in receiptThis addresses previous critical feedback about receipt status validation.
88-95: LGTM! Account info query is correctly implemented.The function properly constructs and executes an
AccountInfoQueryusing the standard SDK pattern.
107-117: LGTM! Main function correctly orchestrates the refactored flow.The main function properly:
- Sets up the client
- Generates keys
- Creates the account with explicit public/private key separation
- Fetches and displays account info
- Handles errors with clear messaging and proper exit codes
This refactoring successfully addresses the objectives from issue #1321 by splitting the monolithic function into clearly named, modular components.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
CHANGELOG.md (1)
10-10: Move this refactor entry to the "Changed" section.Refactoring existing code belongs under "### Changed", not "### Added". The "Added" section is for new features or functionality that didn't exist before.
🔎 Proposed fix
Remove this line from the "### Added" section and place it under "### Changed" (after line 14):
### Added -- Refactored `account_create_transaction_without_alias.py` into smaller, modular functions. - Beginner issue documentation and updated GFI and GFIC templates and documentationThen add it under the "### Changed" section:
### Changed +- Refactored `account_create_transaction_without_alias.py` into smaller, modular functions. - Added unit test for 'endpoint.py' to increase coverage.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.mdexamples/account/account_create_transaction_without_alias.py
🧰 Additional context used
📓 Path-based instructions (1)
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.Priority 1 - Correctness:
- Verify transaction lifecycle chain (construction -> freeze_with -> sign -> execute).
- Ensure
freeze_with(client)is called BEFORE signing.- Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase.- Ensure response validation checks
receipt.statusagainstResponseCodeenums (e.g.,ResponseCode.SUCCESS).Priority 2 - Transaction Lifecycle:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit
.execute(client)calls.- Verify response property extraction (e.g., using
.token_id,.account_id,.serial_numbers).- Ensure error handling uses
ResponseCode(receipt.status).namefor clarity.Priority 3 - Naming & Clarity:
- Enforce role-based naming:
operator_id/_key,treasury_account_id/_key,receiver_id/_key.- Use
_idsuffix for AccountId and_keysuffix for PrivateKey variables.- Validate negative examples explicitly check for failure codes (e.g.,
TOKEN_HAS_NO_PAUSE_KEY).- Ensure logical top-to-bottom flow without ambiguity.
Priority 4 - Consistency:
- Verify standard patterns:
def main(),if __name__ == "__main__":,load_dotenv().- IMPORT RULES:
- Accept both top-level imports (e.g.,
from hiero_sdk_python import PrivateKey) and fully qualified imports (e.g.,from hiero_sdk_python.crypto.private_key import PrivateKey).- STRICTLY validate that the import path actually exists in the project structure. Compare against other files in
/examplesor your knowledge of the SDK file tree.- Flag hallucinations immediately (e.g.,
hiero_sdk_python.keysdoes not exist).- Check for
try-exceptblocks withsys.exit(1)for critical failures.Priority 5 - User Experience:
- Ensure comments explain SDK usage patterns (for users,...
Files:
examples/account/account_create_transaction_without_alias.py
🧬 Code graph analysis (1)
examples/account/account_create_transaction_without_alias.py (3)
src/hiero_sdk_python/account/account_create_transaction.py (2)
AccountCreateTransaction(27-380)set_key_without_alias(94-107)src/hiero_sdk_python/query/account_info_query.py (1)
AccountInfoQuery(11-131)src/hiero_sdk_python/account/account_info.py (1)
AccountInfo(20-208)
🪛 LanguageTool
CHANGELOG.md
[grammar] ~11-~11: Ensure spelling is correct
Context: ... and updated GFI and GFIC templates and documentation - Enable auto assignment to good first issues (#...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
14-14: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🪛 Ruff (0.14.10)
examples/account/account_create_transaction_without_alias.py
14-14: typing.Tuple is deprecated, use tuple instead
(UP035)
70-72: Avoid specifying long messages outside the exception class
(TRY003)
77-79: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: run-examples
- GitHub Check: build-and-test (3.12)
- GitHub Check: build-and-test (3.11)
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.10)
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (6)
examples/account/account_create_transaction_without_alias.py (6)
38-43: LGTM! Client setup follows best practices.The use of
Client.from_env()simplifies initialization and the function provides clear feedback about the network and operator.
45-51: LGTM! Key generation is clear and complete.The function now correctly returns both the private and public key pair, addressing previous review feedback. The implementation is straightforward and educational for users.
53-82: LGTM! Transaction lifecycle and error handling are correct.The implementation demonstrates proper SDK usage:
- Transaction construction with explicit parameters
- Correct use of
set_key_without_alias()with the public key (not private)- Proper lifecycle: freeze → sign → execute
- Response validation with
ResponseCode.SUCCESSbefore accessing properties- Defensive null check for
account_idThis is an excellent example for users to follow.
Based on coding guidelines for examples, Priority 1-2 requirements.
84-91: LGTM! Query implementation is correct.The function demonstrates proper query usage with clear method chaining and appropriate type hints.
93-101: LGTM! Output formatting is clear and informative.The function effectively demonstrates the account information structure, particularly highlighting the
contract_account_idfield which is relevant to the "no alias" scenario.
103-113: LGTM! Main function orchestrates the flow clearly.The function demonstrates a clean, sequential flow that's easy for users to understand and replicate. The error handling provides clear feedback on failure.
4f7c14b to
e5295f6
Compare
|
This is looking great!! Please make sure to resolve merge conflicts. If you have any questions or need assistance please let us know, we are happy to help :) |
Signed-off-by: tech0priyanshu <[email protected]>
Signed-off-by: tech0priyanshu <[email protected]>
Signed-off-by: tech0priyanshu <[email protected]>
Signed-off-by: tech0priyanshu <[email protected]>
Signed-off-by: tech0priyanshu <[email protected]>
b487ced to
a2dad0f
Compare
Signed-off-by: tech0priyanshu <[email protected]>
Signed-off-by: tech0priyanshu <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: run-examples
- GitHub Check: build-and-test (3.10)
- GitHub Check: build-and-test (3.12)
- GitHub Check: build-and-test (3.11)
- GitHub Check: build-and-test (3.13)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: StepSecurity Harden-Runner
Signed-off-by: tech0priyanshu <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: run-examples
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.10)
- GitHub Check: build-and-test (3.12)
- GitHub Check: build-and-test (3.11)
- GitHub Check: StepSecurity Harden-Runner
|
perfect! Thank you |
Description:
Related issue(s):
Fixes #1321
Notes for reviewer:
Checklist