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

feat: multisig join --group arg #782

Merged
merged 2 commits into from
May 14, 2024

Conversation

kentbull
Copy link
Contributor

This adds the capability to specify an alias during multisig join. Previously the alias was always set to "test alias" which would cause alias collision problems when doing successive multisig join commands for different identifiers.

@@ -196,7 +201,12 @@ def incept(self, attrs):

if approve:
if self.auto:
alias = "test alias"
alias = self.alias
Copy link
Member

Choose a reason for hiding this comment

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

This does not account for auto being true and alias being None which will cause an exception below. (As happened in the scripts run for this PR).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was changed in the most recent commit to have a nested if statement checking of the alias (now "group") is set.
For example:

            if self.auto:
                if self.group is None:
                    group = "default-group"
                else:
                    group = self.group

@@ -196,7 +201,12 @@ def incept(self, attrs):

if approve:
if self.auto:
alias = "test alias"
alias = self.alias
elif self.alias:
Copy link
Member

Choose a reason for hiding this comment

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

self.alias should only be used if self.auto is True.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was changed in the most recent commit to have a nested if statement checking of the alias (now "group") is set.
For example:

            if self.auto:
                if self.group is None:
                    group = "default-group"
                else:
                    group = self.group

Copy link
Member

@pfeairheller pfeairheller left a comment

Choose a reason for hiding this comment

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

Please fix the logic so that the alias is only used if self.auto is True. I would also recommend using a different argument name as --alias has specific meaning in most other commands.

@kentbull
Copy link
Contributor Author

I changed the argument name from --alias to be --group to align with the argument name sent in to self.hby.makeGroupHab.

@kentbull kentbull changed the title feat: multisig join --alias arg feat: multisig join --group arg May 14, 2024
@pfeairheller pfeairheller merged commit 77b1d43 into WebOfTrust:v1.1.14 May 14, 2024
6 checks passed
@kentbull kentbull deleted the kb/multisig-join-alias branch May 14, 2024 23:14
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.

None yet

2 participants