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

Draft: Wizard #145

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

DanielVoogsgerd
Copy link
Member

Note: this is still very much a draft. Feel free to look around if you are interested.
Suggestions are welcome as always, but a lot of stuff still needs to be written, rewritten, and restructured.

I do want some sort of place where progress can be seen and tracked so this will be just that.

To Do:

  • Resolve or remove to do comments.
  • Add token generation
  • Restructure weird namespacing using the Wizard enum
  • Move defaults to specifications crate

Did I mention it is a draft? 😆

Copy link
Member

@Lut99 Lut99 left a comment

Choose a reason for hiding this comment

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

Looks good so far! Keep at it ;)

@@ -3,13 +3,14 @@ name = "brane-ctl"
version = { workspace = true }
edition = "2021"
authors = [ "Tim Müller" ]
Copy link
Member

Choose a reason for hiding this comment

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

By the way! Unrelated to this PR, but don't be afraid to add yourself as author ;)


// Resolve any path depending on the '$CONFIG'
let infra: PathBuf = resolve_config_path(infra, &config_path);
let proxy: PathBuf = resolve_config_path(proxy, &config_path);
let certs: PathBuf = resolve_config_path(certs, &config_path);

// Ensure the directory structure is there
// TODO: Does not really seem like the responsibility of generating a node as if these
Copy link
Member

Choose a reason for hiding this comment

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

OK good catch xD it should just be an assert instead, then.

@@ -593,6 +683,7 @@ where
///
/// # Errors
/// This function errors if we failed to interact with the user.
// TODO: This should be display preferable I would think
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -89,6 +139,8 @@ impl Address {
/// # Returns
/// A new Address instance.
#[inline]
// TODO: Maybe it is better to just implement From<(Ipv4Addr>, u16)>
// Having n+1 constructors seems a bit excessive
Copy link
Member

Choose a reason for hiding this comment

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

Also fine. I think I did it because I would be too lazy to create an Ipv4Addr the whole time instead of just adding the numbers directly. You can also implement From<((u8, u8, u8, u8), u16> in addition for convenience.

pub const CERTIFICATE_PATH: &str = "certs";
pub const PACKAGE_PATH: &str = "packages";

pub const NODE_HEADER: &str = indoc::indoc!("
Copy link
Member

Choose a reason for hiding this comment

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

What an excellent little crate xD

Not yet finished, but we are getting quite far along.
Some code paths contain todo!() macros and some ports are still defined
to garbage
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

devskim found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

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