-
Notifications
You must be signed in to change notification settings - Fork 412
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
UX improvements for Merge configuration #4529
Conversation
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.
LGTM
@@ -120,7 +131,8 @@ private void EnsureReceiptAvailable() | |||
{ | |||
if (!syncConfig.DownloadReceiptsInFastSync || !syncConfig.DownloadBodiesInFastSync) | |||
{ | |||
throw new InvalidOperationException("Receipt and body must be available for merge to function"); | |||
if (_logger.IsError) _logger.Error("Receipt and body must be available for merge to function. The following configs values should be set to true: Sync.DownloadReceiptsInFastSync, Sync.DownloadBodiesInFastSync"); | |||
_environment.Exit(1); |
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.
I would create some static ExitCodes class and name those + make unique codes, so if we get info from users we can actually diagnose it easily
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.
I would create some static ExitCodes class and name those + make unique codes, so if we get info from users we can actually diagnose it easily
Done :)
public const int NoEngineModule = 1; | ||
|
||
public const int NoDownloadOldReceiptsOrBlocks = 2; |
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.
I would divide the errors in some numbers, for example configuration errors would be between 1_000 and 1_999, wouldn't go from 1 :)
UX improvements for merge configuration + log fix