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: add w3 name get-key command #2107

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

adamalton
Copy link
Contributor

Fixes #1835.

Copy link
Contributor

@joshJarr joshJarr left a comment

Choose a reason for hiding this comment

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

LGTM, great shout.

Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

This feels like half of the solution - how do I then use that key in a different w3 CLI?

Personally I'd call these:

w3 name export <keyId> > priv.key
w3 name import priv.key

@adamalton
Copy link
Contributor Author

@alanshaw having both an import and an export command makes sense. Do you think that these should only be for doing one key at a time, or do you think that it would actually be a nicer user experience to be able to export all of your name keys at once into a single file, and then import them in bulk too? Or do you think that the common use case of w3 name functionality is typically only one or two names at a time?

@alanshaw
Copy link
Member

alanshaw commented Nov 30, 2022

IMO just one at a time...multiple would mean that there has to be a format to the exported file and I don't want to have to define that, or trust users to pick out a particular key from it. You also can't give it directly to something else that is able to read the keys (like programmatically to the library) without also making changes there.

@alanshaw
Copy link
Member

...you could also add w3 name export --all later and augment w3 name import to be able to read one or many...

@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2022

package-lock.json changes

Click to toggle table visibility
Name Status Previous Current
fs ADDED - 0.0.1-security

@adamalton
Copy link
Contributor Author

Now refactored as per Alan's suggestion, and tests added for additional fun and profit.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2022

@adamalton
Copy link
Contributor Author

I think that the failing test here will be fixed by #2151.

@@ -26,6 +26,7 @@
"@ipld/car": "^3.1.16",
"conf": "^10.1.1",
"enquirer": "^2.3.6",
"fs": "^0.0.1-security",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"fs": "^0.0.1-security",

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.

Add w3 name get <keyId> command
5 participants