<p><b>@mrosseel</b> commented on this pull request.</p>

<p>utACK</p>
<p>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 <a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="44837901" data-permission-text="Issue title is private" data-url="https://github.com/bisq-network/bisq/issues/123" data-hovercard-type="issue" data-hovercard-url="/bisq-network/bisq/issues/123/hovercard" href="https://github.com/bisq-network/bisq/issues/123">#123</a>' comment so it doesn't get merged before the parent. Would make for cleaner diffs</p><hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/3086#discussion_r313839912">core/src/test/java/bisq/core/dao/governance/proposal/MyProposalListServiceTest.java</a>:</p>
<pre style='color:#555'>> +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);
</pre>
<p>why is storage mocked separately? It's just passed as an argument like the rest.</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/3086#discussion_r313841825">desktop/src/main/java/bisq/desktop/main/offer/MakerFeeMaker.java</a>:</p>
<pre style='color:#555'>> @@ -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 {
</pre>
<p>not sure about the name, too many makers making things :)<br>
Don't have any good alternatives though... MakerFeeFactory, MakerFeeCalculation, MakerFeeCreator, ... (if you don't find anything better don't worry)</p>
<p>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?</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/bisq-network/bisq/pull/3086?email_source=notifications&email_token=AJFFTNR46SLHADECXAQ5VBTQEP5KDA5CNFSM4ILT5NMKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCBQ36AY#pullrequestreview-274841347">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AJFFTNR56NALFBIC7PKH3MLQEP5KDANCNFSM4ILT5NMA">mute the thread</a>.<img src="https://github.com/notifications/beacon/AJFFTNX3CGJ775OIP3RSFXLQEP5KDA5CNFSM4ILT5NMKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCBQ36AY.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/bisq-network/bisq/pull/3086?email_source=notifications\u0026email_token=AJFFTNR46SLHADECXAQ5VBTQEP5KDA5CNFSM4ILT5NMKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCBQ36AY#pullrequestreview-274841347",
"url": "https://github.com/bisq-network/bisq/pull/3086?email_source=notifications\u0026email_token=AJFFTNR46SLHADECXAQ5VBTQEP5KDA5CNFSM4ILT5NMKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCBQ36AY#pullrequestreview-274841347",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>