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

refactor: use OS-specific config & data dirs, default iroh-store dir #218

Merged
merged 2 commits into from
Sep 7, 2022

Conversation

b5
Copy link
Member

@b5 b5 commented Sep 1, 2022

switch to using https://dirs.dev/ standard application directories instead of $HOME/.iroh

iroh-store now uses a default directory for writing rocksDB files instead of explicitly requiring a store path. Defaults to /iroh/store within the OS application data directory.

closes n0-computer/beetle#231

@b5 b5 force-pushed the feat/dirs_dev branch 3 times, most recently from 1616340 to 2df6113 Compare September 1, 2022 16:32
@b5 b5 requested review from dignifiedquire and Arqu September 1, 2022 16:40
Comment on lines +23 to +26
match arg_path {
Some(p) => Ok(p),
None => iroh_data_path("store"),
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm unclear on weather or not there's a more succinct way to express this

Copy link
Contributor

Choose a reason for hiding this comment

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

I think arg_path.map_or(iroh_data_path("store"), |v| Ok(v)) is equivalent - but not easier to read!

Copy link
Member Author

Choose a reason for hiding this comment

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

oooh I hadn't found map_or. Thanks for the tip @fabricedesre! I agree with you that the match is easier to read, and will leave it as is unless I'm overruled by dig 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

seems fine and readable

@b5 b5 requested a review from dignifiedquire September 2, 2022 03:03
dignifiedquire
dignifiedquire previously approved these changes Sep 5, 2022
Copy link
Contributor

@dignifiedquire dignifiedquire left a comment

Choose a reason for hiding this comment

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

lgtm

Arqu
Arqu previously approved these changes Sep 7, 2022
Copy link
Collaborator

@Arqu Arqu 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 from the ops side too. All of the deployment stuff is hardwired to avoid sudden config changes like this.

b5 added 2 commits September 7, 2022 11:28
switch to using https://dirs.dev standard application directories instead of $HOME/.iroh

iroh-store now uses a default directory for datastore data instead of explicitly requiring a store path
@b5 b5 dismissed stale reviews from Arqu and dignifiedquire via 4f6c296 September 7, 2022 15:28
@b5 b5 requested review from dignifiedquire and Arqu September 7, 2022 17:00
Copy link
Collaborator

@Arqu Arqu left a comment

Choose a reason for hiding this comment

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

👍

@Arqu Arqu merged commit 5cfb907 into main Sep 7, 2022
@Arqu Arqu deleted the feat/dirs_dev branch September 7, 2022 20:07
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.

Improve default paths
4 participants