-
Notifications
You must be signed in to change notification settings - Fork 17
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
Made Quai CPU Miner a CLI app #12
Conversation
To go with the new cli app, have added 2 flags, region and zone. Main objective of this refactor was the ease of sanity checks and inheriting all the cool features available in the cli library. Before we start integrating new proxy and other features into the Miner wanted to make the Miner a cli app.
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.
#11 is a large PR that this will create conflicts with. I recommend we merge that first and then work to resolve.
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.
Related to using helper consts from the common library
} | ||
if !ctx.IsSet(ZoneFlag.Name) { | ||
return errors.New("zone flag is not set") | ||
} else if ctx.Int(ZoneFlag.Name) < 0 || ctx.Int(ZoneFlag.Name) > 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.
Same as ^
// Appropriate errors | ||
if !ctx.IsSet(RegionFlag.Name) { | ||
return errors.New("region flag is not set") | ||
} else if ctx.Int(RegionFlag.Name) < 0 || ctx.Int(RegionFlag.Name) > 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.
Use helper common.HierarchyDepth
if !ctx.IsSet(RegionFlag.Name) { | ||
return errors.New("region flag is not set") | ||
} else if ctx.Int(RegionFlag.Name) < 0 || ctx.Int(RegionFlag.Name) > 2 { | ||
return errors.New("current ontology prime only has 3 regions, so please input between 0 and 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.
In the messages too
if !ctx.IsSet(ZoneFlag.Name) { | ||
return errors.New("zone flag is not set") | ||
} else if ctx.Int(ZoneFlag.Name) < 0 || ctx.Int(ZoneFlag.Name) > 2 { | ||
return errors.New("current ontology each region only has 3 zones, so please input between 0 and 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.
Same as ^
To go with the new cli app, have added 2 flags, region and zone.
Main objective of this refactor was the ease of sanity checks and inheriting all the cool features available in the cli library. Before we start integrating new proxy and other features into the Miner wanted to make the Miner a cli app.
Resolves #4