-
Notifications
You must be signed in to change notification settings - Fork 40
fix: improve build spec error messaging #477
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
Conversation
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## main #477 +/- ##
==========================================
+ Coverage 75.11% 75.18% +0.06%
==========================================
Files 70 71 +1
Lines 14967 15053 +86
Branches 14967 15053 +86
==========================================
+ Hits 11243 11318 +75
- Misses 2253 2255 +2
- Partials 1471 1480 +9
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
chungquantin
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.
Seems this PR's issue is already handled in the benchmarking. As benchmarking is not merged to main yet, my suggestion is to have a common method that can be used for both features and then I'll refactor the benchmarking feature after it is merged to main.
chungquantin
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.
Looks great!
This PR improves the error message when building specs fails, making it clearer and more aligned with the output of the underlying command.
Before:
Running
pop build spec --chain ./wrong-chain-spec:This error message was not very informative and did not clearly indicate the root cause of the failure.
Now:
This message explicitly states the failure reason and matches the output of the underlying command:
Closes #475
[sc-3371]