-
Notifications
You must be signed in to change notification settings - Fork 35
feat(CLI)!: Split iota names set-target-address
#6878
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
base: develop
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
set-target-address
Ok(match self { | ||
IotaNamesNftProxy::Domain(_) => { | ||
let names_config = get_iota_names_config(client).await?; | ||
names_config.package_address.into() |
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.
the main package?
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.
Yeah that's where the controller module is.
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.
But you're not using the controller module anymore?
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.
No, I am now using it for non-subdomains. For the subdomains it uses the subdomains
module
@@ -1356,6 +1415,13 @@ impl IotaNamesNftProxy { | |||
} | |||
|
|||
fn module_name(&self) -> &'static str { | |||
match self { | |||
IotaNamesNftProxy::Domain(_) => "controller", |
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.
Tbh this feels weird, very tailored to solve this PR's issue
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.
Yeah, I know. But how would you rather do 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.
I think we need to either:
- resolve the module name when it's actually needed; or
- rename this function
module_name
is too broad for this, maybecontroller_module_name
or something assubdomain_proxy
essentially forward to the controller anyway
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.
Also applies to package_id
though, I think I need to think a bit more about it, I don't like it
Description of change
Splits the
set-target-address
command intounset-target-address
. Theset-target-address
will now default to the active address when calling without the optional param.Links to any relevant issues
Closes https://github.com/iotaledger/iota-names/issues/190
Type of change
Choose a type of change, and delete any options that are not relevant.
Release Notes
set-target-address
command intoset-target-address
andunset-target-address
.