[bisq-network/bisq] inject publickeyring (#3086)

[mrosseel]

mrosseel commented on this pull request.

utACK

Don't know how the 'mergers' think about this but wouldn't it be cleaner if you rebase this on your previous PR (this PR doesn't only contain the inject publickeyring changes) ? You could add a 'blocked by #123' comment so it doesn't get merged before the parent. Would make for cleaner diffs

> +import bisq.common.crypto.PubKeyRing;
+import bisq.common.storage.Storage;
+
+import javafx.beans.property.SimpleIntegerProperty;
+
+import org.junit.Test;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class MyProposalListServiceTest {
+    @Test
+    public void canInstantiate() {
+        P2PService p2PService = mock(P2PService.class);
+        when(p2PService.getNumConnectedPeers()).thenReturn(new SimpleIntegerProperty(0));
+        Storage storage = mock(Storage.class);

why is storage mocked separately? It's just passed as an argument like the rest.

> @@ -0,0 +1,13 @@
+package bisq.desktop.main.offer;
+
+import bisq.core.btc.wallet.BsqWalletService;
+import bisq.core.offer.OfferUtil;
+import bisq.core.user.Preferences;
+
+import org.bitcoinj.core.Coin;
+
+public class MakerFeeMaker {

not sure about the name, too many makers making things :)
Don't have any good alternatives though... MakerFeeFactory, MakerFeeCalculation, MakerFeeCreator, ... (if you don't find anything better don't worry)

This is just a wrapper for one of the util classes, so will you need to do this for every method in every Util class? Are you trying to avoid calls to static methods for unit testing purposes?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/3086#pullrequestreview-274841347
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20190814/01bb7c88/attachment.html>