Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

Signing 2 - hc keygen #974

Merged
merged 19 commits into from Feb 11, 2019
Merged

Signing 2 - hc keygen #974

merged 19 commits into from Feb 11, 2019

Conversation

lucksus
Copy link
Collaborator

@lucksus lucksus commented Feb 8, 2019

  • I have added a summary of my changes to the changelog

hc keygen asks for passhprase and writes encrypted key bundle file to ~/.holochain/keys.

.unwrap();

let path = match directories::UserDirs::new() {
Some(user_dirs) => user_dirs.home_dir().join(".holochain").join("keys"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Based on what I added into the tree about directories, I propose that we create a new crate, or shareable file, that keeps track of and documents all the “magic strings” representing paths on devices. I find that they get scattered in the code and the act as high risk points of failure for our system I think. If we could start in that direction in this Pr that would be amazing

Copy link
Contributor

Choose a reason for hiding this comment

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

Also I'm in favor of creating these directories with the XDG_CONFIG convention in mind. The directories crate already does that for you, we just have to use it.

Copy link
Contributor

@sphinxc0re sphinxc0re left a comment

Choose a reason for hiding this comment

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

Remarks aside, the unification of paths does look good

common/src/paths.rs Outdated Show resolved Hide resolved
common/src/paths.rs Outdated Show resolved Hide resolved
extern crate bip39;
extern crate boolinator;
extern crate rustc_serialize;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed again?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know. I've just moved this by two lines for alphanumeric sorting.

Copy link
Collaborator

@Connoropolous Connoropolous left a comment

Choose a reason for hiding this comment

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

tests?

@Connoropolous
Copy link
Collaborator

Also, this needs to be added to the readme of CLI, plus the mdbook.

sphinxc0re and others added 4 commits February 11, 2019 16:22
common/src/paths.rs Outdated Show resolved Hide resolved
Co-Authored-By: lucksus <nicolas@lucksus.org>
@Connoropolous
Copy link
Collaborator

Ok, this is great I think, AND it takes us for now to a point where there is a ~/.hc ~/.holochain and now additionally ~/.config/holochain ... I guess this is with the intention to press forward on making this consistent, and the XDG compliant approach would be the one we'd consolidate to?

cli/src/main.rs Outdated Show resolved Hide resolved
Co-Authored-By: lucksus <nicolas@lucksus.org>
@lucksus
Copy link
Collaborator Author

lucksus commented Feb 11, 2019

@sphinxc0re, ok to merge?

@Connoropolous
Copy link
Collaborator

@lucksus can you comment on #974 (comment)?

@lucksus
Copy link
Collaborator Author

lucksus commented Feb 11, 2019

@Connoropolous, yes, this only makes sense if we now switch all config path handling to holochain_common::paths. But please let us do that in a follow-up PR.

@lucksus lucksus merged commit 310c924 into develop Feb 11, 2019
@zippy zippy deleted the hc-keygen branch July 4, 2019 19:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants