Skip to content
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: error messages to be concise and formatted correctly #40 #184

Closed
wants to merge 1 commit into from
Closed

Refactor: error messages to be concise and formatted correctly #40 #184

wants to merge 1 commit into from

Conversation

AdamuAbba
Copy link

@AdamuAbba AdamuAbba commented Apr 26, 2024

closes issue #40

This PR contains minor code changes that modify the structure of some error messages. I also did not modify some "one-liners" that seemed convenient enough. I'd appreciate a review and any pointers for adjustments thank you.

@AdamuAbba AdamuAbba changed the title refactor error messages Refactor: Refactor error messages issue #40 Apr 26, 2024
@AdamuAbba AdamuAbba changed the title Refactor: Refactor error messages issue #40 Refactor: error messages to be concise and formatted correctly #40 Apr 26, 2024
Copy link
Contributor

@carlaKC carlaKC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm hesitant to just add newlines to our errors for a marginal improvement in formatting - seems like somewhere we'll quickly get inconsistent and forget so I'd like to find a more sustainable way to approach this.

@@ -171,7 +171,7 @@ pub struct ActivityDefinition {

#[derive(Debug, Error)]
pub enum SimulationError {
#[error("Lightning Error: {0:?}")]
#[error("Lightning Error")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this just swallow the actual error that happened? I still think we should surface that information, and then handle it in a prettier way in the cli if required?

@@ -13,7 +13,7 @@ const SECONDS_PER_MONTH: u64 = HOURS_PER_MONTH * 60 * 60;

#[derive(Debug, Error)]
pub enum RandomActivityError {
#[error("Value error: {0}")]
#[error("Value error\nCaused by: {0}")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have Caused by here but not below?

@carlaKC
Copy link
Contributor

carlaKC commented May 24, 2024

Closing due to inactivity, feel free to re-open if you've got an update here.

@carlaKC carlaKC closed this May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants