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

Test guice setup and fix #3043

Merged
merged 5 commits into from
Aug 5, 2019

Conversation

christophsturm
Copy link
Contributor

@christophsturm christophsturm commented Aug 1, 2019

@sqrrm's guice fix and a unit test that checks seednode's guice setup. superseedes #3041

Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

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

ACK

build.gradle Outdated Show resolved Hide resolved
@christophsturm christophsturm force-pushed the test-guice-setup-and-fix branch from e36f36f to c1a29c2 Compare August 1, 2019 14:00
@christophsturm christophsturm force-pushed the test-guice-setup-and-fix branch from 58e867a to f96edd2 Compare August 1, 2019 14:20
@@ -97,6 +97,7 @@ public String toString() {

@Inject
public KeyStorage(@Named(KEY_STORAGE_DIR) File storageDir) {
storageDir.mkdirs();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the CI error came from a missing key directory on CI. I now changed the keystorage to create the directory if its missing. not sure if that has any downsides.

@christophsturm
Copy link
Contributor Author

note that i had to bump guice to get a useful error message. I removed the guice bump again from this pr but we really should consider updating guice.

Copy link
Contributor

@freimair freimair left a comment

Choose a reason for hiding this comment

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

Ack

@christophsturm
Copy link
Contributor Author

can we get this merged?

Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

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

utACK

@sqrrm sqrrm merged commit 276a506 into bisq-network:master Aug 5, 2019
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.

3 participants