chore: format ethereum_transaction_test.py with black#1563
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 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. WalkthroughAdds two unit tests for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Pull request overview
This PR formats the ethereum_transaction_test.py file using the black code formatter to maintain consistent code style across the test suite, addressing issue #1528.
Changes:
- Added blank lines between test functions to comply with black formatting standards
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/unit/ethereum_transaction_test.py | Added blank lines after test functions to meet black formatter requirements |
| .changes/format-ethereum-transaction-test.md | Added changelog entry documenting the formatting change |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.changes/format-ethereum-transaction-test.md (1)
3-3: Remove the backslashes before the dash and underscores.The escaped dash and underscores prevent proper markdown rendering. This issue was previously flagged for the underscores.
📝 Proposed fix
-\- Formatted ethereum\_transaction\_test.py using black. +- Formatted ethereum_transaction_test.py using black.
AntonioCeppellini
left a comment
There was a problem hiding this comment.
HI @shrav-jally thanks for your contribution :D. this is looking great!!
See this https://github.com/hiero-ledger/hiero-sdk-python/blob/main/docs/sdk_developers/training/workflow/09_changelog_entry.md in order to properly add your entry into the CHANGELOG.md
94e3ae2 to
7ce6e32
Compare
exploreriii
left a comment
There was a problem hiding this comment.
Hi @shrav-jally
Unfortunately, we do require a changelog entry
There is a test section under UNRELEASED - that should be the most appropriate one :)
|
Hi @exploreriii, thanks for the guidance! |
|
Hi @shrav-jally, thanks for the update 🙂 I’m not seeing any changes in CHANGELOG.md yet. Have you pushed the changelog updates to the branch? |
fa5950e to
4bbfbba
Compare
|
Hi @manishdait, thanks for checking |
|
Hi, this is WorkflowBot.
|
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1563 +/- ##
=======================================
Coverage 92.89% 92.89%
=======================================
Files 140 140
Lines 8765 8765
=======================================
Hits 8142 8142
Misses 623 623 🚀 New features to boost your workflow:
|
beaaa33 to
34cb561
Compare
|
Thank you for your patience and for guiding me through this, I really appreciate the help and feedback throughout the process |
34cb561 to
80e4037
Compare
|
Hi, this is MergeConflictBot. Please resolve these conflicts locally and push the changes. To assist you, please read: Thank you for contributing! |
1 similar comment
|
Hi, this is MergeConflictBot. Please resolve these conflicts locally and push the changes. To assist you, please read: Thank you for contributing! |
80e4037 to
6fc546b
Compare
|
Thanks for the clear pointer! I’ve deleted the specified lines and removed the remaining conflict markers from CHANGELOG.md. |
8994999 to
7c7b3a4
Compare
ed022ec to
f412f61
Compare
|
Yes I have force pushed. |
exploreriii
left a comment
There was a problem hiding this comment.
Your fork is 20 commits behind the main
https://github.com/shrav-jally/hiero-sdk-python
I think this is causing your changelog to appear like you are deleting some lines
https://github.com/hiero-ledger/hiero-sdk-python/pull/1563/changes
Please can you follow the prior instructions to pull upstream changes and make it an identical copy
and then rebase your main and force push
see rebasing.md and merge_conflicts.md
Signed-off-by: Shravya Jallepally <jallepally0.0shravya@gmail.com>
Signed-off-by: Shravya Jallepally <jallepally0.0shravya@gmail.com>
Signed-off-by: Shravya Jallepally <jallepally0.0shravya@gmail.com>
b783ec6 to
88151dc
Compare
|
Could you please take another look when you have a moment? |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/ethereum_transaction_test.py (1)
249-253: Consider adding a type assertion for the receipt.Per the coding guidelines (Priority 1), asserting return types helps protect against breaking changes. Adding a type check ensures the return contract is maintained.
💡 Suggested improvement
receipt = transaction.execute(client) + assert receipt is not None, "execute() should return a receipt" assert ( receipt.status == ResponseCode.SUCCESS ), "Transaction should have succeeded"
Your latest commit has you removing two changes from the changelog instead of adding one for this change. Please reinstate those changes and add your change. |
88151dc to
bc89921
Compare
Signed-off-by: Shravya Jallepally <jallepally0.0shravya@gmail.com>
bc89921 to
c679dfe
Compare
|
Approved let's hope the checks pass then we can. merge this, thank you |
|
All checks are passing and approvals are in. |
Fixes #1528