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

fix build on jdk11 and jdk12. remove powermock and jmockit #3081

Merged
merged 10 commits into from
Aug 14, 2019

Conversation

christophsturm
Copy link
Contributor

@christophsturm christophsturm commented Aug 13, 2019

on jdk 12 the app still throws a NPE at startup (but afterwards it seems to work), but runs fine on jdk11.

build and test suite works on jdk10,11,12, inside IDEA or with ./gradlew clean build

jdk 10 is probably still needed for binaries. but contributors can now use any jdk 10+ that they like

had to disable a very small amount of tests (i think 3) that were not so easy to convert.

Copy link

@blabno blabno left a comment

Choose a reason for hiding this comment

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

Great job. I have only one small remark.

@Before
public void setup() throws IOException {

tsm = mock(TradeStatisticsManager.class);
Copy link

Choose a reason for hiding this comment

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

Please avoid acronyms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah that's what it was called before, will change.

Copy link

@blabno blabno left a comment

Choose a reason for hiding this comment

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

utACK

@@ -51,7 +51,7 @@
@EqualsAndHashCode
@Getter
@Slf4j
public final class OfferPayload implements ProtectedStoragePayload, ExpirablePayload, RequiresOwnerIsOnlinePayload {
public class OfferPayload implements ProtectedStoragePayload, ExpirablePayload, RequiresOwnerIsOnlinePayload {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should stay final for security reasons

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is final (not in this commit but later)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah saw it now... thanks.


import org.bitcoinj.core.Coin;

public class MakerFeeMaker {
Copy link
Contributor

Choose a reason for hiding this comment

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

MakerFeeMaker sounds a bit weird. Maybe MakerFeeProvider?

Copy link
Contributor

@ManfredKarrer ManfredKarrer 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 e010ba8 into bisq-network:master Aug 14, 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.

4 participants