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

Add a builder method to customize where logs are stored #129

Merged
merged 2 commits into from
Jul 6, 2023

Conversation

orbitalturtle
Copy link
Contributor

@orbitalturtle orbitalturtle commented Jun 20, 2023

Just a small PR to add the option to customize where logs are stored.

Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

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

Thanks for having a look at this!

bindings/ldk_node.udl Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/builder.rs Outdated Show resolved Hide resolved
src/builder.rs Outdated Show resolved Hide resolved
@tnull
Copy link
Collaborator

tnull commented Jun 21, 2023

Btw, as you already are touching logging stuff, would you be interested in picking up #126 (as a separate commit)?

@orbitalturtle
Copy link
Contributor Author

@tnull Sure thing! I'll take a look at that symlink issue tomorrow

@orbitalturtle orbitalturtle force-pushed the set-log-dir branch 3 times, most recently from 569e1e2 to 8a587e9 Compare June 23, 2023 01:08
@orbitalturtle
Copy link
Contributor Author

@tnull Made those changes

@orbitalturtle orbitalturtle requested a review from tnull June 23, 2023 01:30
src/builder.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/logger.rs Outdated Show resolved Hide resolved
@orbitalturtle
Copy link
Contributor Author

Oh I messed something up, will fix before you do another pass.

@orbitalturtle
Copy link
Contributor Author

@tnull ok fixed :)

Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

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

Excuse the delay here!

LGTM, one really minor doc nit. Other than that it's good to go. :)

src/lib.rs Outdated Show resolved Hide resolved
@orbitalturtle
Copy link
Contributor Author

@tnull Great, thanks for the reviews! Made that last little change.

@orbitalturtle orbitalturtle requested a review from tnull July 5, 2023 22:16
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

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

LGTM!

@tnull tnull merged commit 5a59240 into lightningdevkit:main Jul 6, 2023
3 checks passed
@tnull tnull mentioned this pull request Jul 11, 2023
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