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.

Sign up for GitHub

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

Jump to bottom

Make persisted file names configurable #442

Merged
merged 1 commit into from
Aug 21, 2023
Merged

Make persisted file names configurable #442

merged 1 commit into from
Aug 21, 2023

Conversation

AArnott
Copy link
Contributor

@AArnott AArnott commented Aug 19, 2023

This allows the zingolib to be used by other applications without them having to share a wallet file.

zancas
zancas reviewed Aug 19, 2023
View reviewed changes
zingolib/src/lib.rs Outdated Show resolved Hide resolved
@AArnott
Copy link
Contributor Author

AArnott commented Aug 19, 2023

I'm not sure why an audit of dependencies failed with my change, which doesn't change dependencies. Is there any work I need to do here?

@zancas
Copy link
Member

zancas commented Aug 20, 2023

We'll @juanky201271 's approval once we're sure our zingo-pc and zingo-mobile dependencies are compatible.

juanky201271
juanky201271 requested changes Aug 20, 2023
View reviewed changes
zingoconfig/src/lib.rs Outdated Show resolved Hide resolved
@juanky201271
Copy link
Contributor

@AArnott can you update your fork because it is 123 commit behind, and I get weird errors when I try to build it in zingo-mobile?

@AArnott
Copy link
Contributor Author

AArnott commented Aug 20, 2023

I've rebased and force pushed, @juanky201271

@AArnott
Make persisted file names configurable
c1dc689 
This allows the zingolib to be used by other applications without them having to share a wallet file.
juanky201271
juanky201271 previously approved these changes Aug 20, 2023
View reviewed changes
@juanky201271
Copy link
Contributor

I built the rust libs for zingo-mobile & zingo-pc and both were working fine without any change in the code. Great!!! @AArnott

@juanky201271 juanky201271 self-requested a review August 20, 2023 21:26
@juanky201271 juanky201271 dismissed their stale review August 20, 2023 21:27

I just see a problem in zingo-mobile, maybe it is not related, but I nned to review it.

@juanky201271
Copy link
Contributor

zingo-pc is working well.

But zingo-mobile is not, when I run the command sync I got this: task 4754 panicked... and I don't know if this is related with this PR or maybe with the last major PR about shardtree...

@AArnott If you build zingo-cli in this branch and you run sync you obtain the same error?

@AArnott
Copy link
Contributor Author

AArnott commented Aug 20, 2023

sync works for me on this branch under zingo-cli.

@juanky201271
Copy link
Contributor

sync works for me on this branch under zingo-cli.

I think my issue is not related with this PR, approving

juanky201271
juanky201271 approved these changes Aug 21, 2023
View reviewed changes
Copy link
Contributor

@juanky201271 juanky201271 left a comment

Choose a reason for hiding this comment

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

looks great.

@juanky201271 juanky201271 merged commit eeca8cf into zingolabs:dev Aug 21, 2023
7 of 15 checks passed
@AArnott AArnott deleted the changeableFileNames branch August 21, 2023 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zancas zancas zancas left review comments

@juanky201271 juanky201271 juanky201271 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AArnott @zancas @juanky201271