fix: align filename and improve error handling in token freeze example#1434
Conversation
Signed-off-by: Shruti Joshi <shrutisjoshi3110@gmail.com>
WalkthroughThe changes refactor Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Signed-off-by: Shruti Joshi <shrutisjoshi3110@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.changes/1412.fix.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
.changes/1412.fix.md
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
⏰ 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). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
.changes/1412.fix.md (1)
1-1: Request the actual changed file for comprehensive review.The PR modifies
examples/tokens/token_freeze_transaction.pywith error handling improvements and filename corrections, but this file is not included in the review context. To fully assess the changes, please provide the modified Python file so I can verify:
- That incorrect run instructions referencing
token_freeze.pyhave been removed- That the exception handling has been properly broadened from
RuntimeErrortoException- That error messages and exit behavior are clear and appropriate
- That the changes align with the PR objectives and don't introduce new issues
exploreriii
left a comment
There was a problem hiding this comment.
Hi @ShrutiJ3110
Thanks for the PR
We require a changelog entry to CHANGELOG.md and not a separate markdown file
Signed-off-by: Shruti Joshi <shrutisjoshi3110@gmail.com>
|
Thanks for the clarification! I’ve moved the changelog entry to CHANGELOG.md and removed the separate markdown file as requested. |
|
Hi, this is MergeConflictBot. Please resolve these conflicts locally and push the changes. To assist you, please read: Thank you for contributing! |
exploreriii
left a comment
There was a problem hiding this comment.
Hi @ShrutiJ3110
Yes please!
Keep one set of run instructions please so a future user knows how to execute this example 👍
aceppaluni
left a comment
There was a problem hiding this comment.
@ShrutiJ3110 This is looking great! 👍
Please make sure to resolve merge conflicts. I linked our Merge-Conflicts.md file for assistance.
If you have questions or need more support, please reach out.
|
Reopening as confirmed working on it |
Signed-off-by: Shruti Joshi <shrutisjoshi3110@gmail.com>
Signed-off-by: Shruti Joshi <shrutisjoshi3110@gmail.com>
MonaaEid
left a comment
There was a problem hiding this comment.
Alot of the changes in the example script aren’t required
|
You’ve handled the error handling and run instructions requirements successfully, but you changed something else that should have remained the same |
exploreriii
left a comment
There was a problem hiding this comment.
Hi @ShrutiJ3110
This PR solves the issue! Well done
As mentioned you have gone beyond the scope of the issue requirements, which is against our workflow
You have:
- deleted run instructions
- changed set up handling
- removed various comments
- also made some improvements
We do agree improvements can be great, but to keep things easy to review and everyone in accordance with what should be done, each submission should have an issue linked to it. That means, if you want to improve a file (and there is no issue already for it), please create an issue, then create your pull request.
For this PR, it should thus ideally only change error handling - nothing else to operator or comments.
Can you please:
- add a changelog entry in the correct fixed location for unreleased
- restore lines 1 to 2, which are crucial instructions on how to run the file for new starters
Once this is done, in this case I will accept the other changes, once.
Thank you
Signed-off-by: Shruti Joshi <shrutisjoshi3110@gmail.com>
|
Thanks for the feedback! |
|
Hi, this is WorkflowBot.
|
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1434 +/- ##
=======================================
Coverage 92.89% 92.89%
=======================================
Files 140 140
Lines 8765 8765
=======================================
Hits 8142 8142
Misses 623 623 🚀 New features to boost your workflow:
|
|
Hi @ShrutiJ3110 , This is super awesome work!! Can you check your changelog? There seems to be an error. |
Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
Aligned token freeze example filename references and improved error handling. Signed-off-by: exploreriii <133720349+exploreriii@users.noreply.github.com>
Fixes #1412