-
Notifications
You must be signed in to change notification settings - Fork 53
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
feat(node-manager): ✨ adds maintain cli command #2119
feat(node-manager): ✨ adds maintain cli command #2119
Conversation
Left this as draft since I'm not a huge rust developer and not 100% familiar with the project layout. The intention is to add the ability to the |
Hi Anthony, Thanks for the contribution! Could you elaborate more please on how you envision this command being used? Cheers, Chris |
@jacderida We want to host nodes for individuals (my parent company is NerdNode but building for DeEEP Network also). As such, with the current |
I see, thanks! Let me take a look at this in a little more detail. I might need to wait until the weekend, but, it looks like a good feature. |
@@ -38,7 +38,7 @@ pub trait Component { | |||
/// # Returns | |||
/// | |||
/// * `Result<()>` - An Ok result or an error. | |||
#[expect(unused_variables)] | |||
#[allow(unused_variables)] |
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.
can we leave these as expect
please, otherwise we can drop the lint
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'm not too familiar with rust, but I couldn't cargo build
with those remaining as expect
. Unless there's some cargo config settings that need to be set?
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.
Hey, I think it might be because it was a new feature that was stablizied in the latest rust version. Could you run rustup update
&& rustup default stable
and try compiling it?
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.
It appears that it does allow me to build but results in a lot more orange and red in my IDE.. Also is there a way to force a rustc version so that it warns others before attempting to build? -- Looks like maybe
[package]
rust-version = "1.81.0"
thanks for this @anthonyra . QQ: In the current setup what's missing is Just wondering if another command is required here vs a flag to auto start the correct node counts. |
In the current setup, the CLI doesn't have the
I thought about this also but it's a hybrid of the |
Hi @anthonyra I really do appreciate the PR here, but, the node manager is going to have a significant feature removed, and so the code in this PR might diverge away too much. For now, we are going to close it, but, we may reopen it in the future. I'm sorry for how long we took to get back to you on this. We've been very busy trying to hit our release schedule. |
Description
Please provide a brief description of the changes made in this pull request. Highlight the purpose of the changes and the problem they address.
Related Issue
None
Type of Change
Please mark the types of changes made in this pull request.
--path
flag and aligning the arguments with the functions.Checklist
Please ensure all of the following tasks have been completed: