cli: use thiserror for CommandError Display #317

NunoAlexandre · 2020-03-31T15:08:40Z

Closes #308

I thought we had more error types in the CLI in need of this but it turns out that only CommandError was missing.

cli/src/lib.rs

CodeSandwich · 2020-04-01T07:03:01Z

It's good to have a function that catches errors on the top-most level in main.rs and nicely formats them in a loop unpacking and printing sources, e.g. each layer of causality in one line, maybe prefixed with caused by or a growing indentation or an arrow.

Make them more granular for reads and writes and don't prefix their display message with "Account storage error".

cli/src/account_storage.rs

Plus some other improvements.

cli/src/account_storage.rs

CodeSandwich

Let's not drag this any longer. For me it's already good, we can discuss and make it perfect for everybody later.

NunoAlexandre requested a review from a team March 31, 2020 15:08

NunoAlexandre self-assigned this Mar 31, 2020

NunoAlexandre force-pushed the cli/thiserror branch from 09da95e to d2a303c Compare March 31, 2020 15:09

CodeSandwich requested review from CodeSandwich and removed request for a team March 31, 2020 15:14

CodeSandwich suggested changes Mar 31, 2020

View reviewed changes

cli/src/lib.rs Outdated Show resolved Hide resolved

cli/src/lib.rs Show resolved Hide resolved

cli/src/lib.rs Outdated Show resolved Hide resolved

geigerzaehler reviewed Mar 31, 2020

View reviewed changes

cli/src/lib.rs Outdated Show resolved Hide resolved

NunoAlexandre force-pushed the cli/thiserror branch from d2a303c to 23af600 Compare April 2, 2020 11:37

NunoAlexandre and others added 2 commits April 2, 2020 13:39

cli: use thiserror for CommandError Display

1e42032

cli: Improve account storage error messages

87764b1

Make them more granular for reads and writes and don't prefix their display message with "Account storage error".

NunoAlexandre force-pushed the cli/thiserror branch from 23af600 to 87764b1 Compare April 2, 2020 11:40

NunoAlexandre requested review from geigerzaehler and CodeSandwich April 2, 2020 12:47

geigerzaehler approved these changes Apr 2, 2020

View reviewed changes

CodeSandwich reviewed Apr 2, 2020

View reviewed changes

cli/src/account_storage.rs Show resolved Hide resolved

cli: Use thiserror #[from] instead of derive_more

5239376

Plus some other improvements.

NunoAlexandre force-pushed the cli/thiserror branch from 353bca0 to 5239376 Compare April 2, 2020 14:44

CodeSandwich reviewed Apr 2, 2020

View reviewed changes

cli/src/account_storage.rs Show resolved Hide resolved

NunoAlexandre requested a review from CodeSandwich April 2, 2020 15:07

CodeSandwich approved these changes Apr 2, 2020

View reviewed changes

NunoAlexandre merged commit d7221a8 into master Apr 2, 2020

NunoAlexandre deleted the cli/thiserror branch April 2, 2020 16:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli: use thiserror for CommandError Display #317

cli: use thiserror for CommandError Display #317

NunoAlexandre commented Mar 31, 2020

CodeSandwich commented Apr 1, 2020

CodeSandwich left a comment

cli: use thiserror for CommandError Display #317

cli: use thiserror for CommandError Display #317

Conversation

NunoAlexandre commented Mar 31, 2020

CodeSandwich commented Apr 1, 2020

CodeSandwich left a comment

Choose a reason for hiding this comment