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

Revert "Apply kotlin plugin and convert one unused class to kotlin" #3235

Closed

Conversation

christophsturm
Copy link
Contributor

This reverts commit 26c053d.

it was decided that kotlin needs to be removed. (btw the kotlin stdlib has been part of bisq for ages because of the netlayer tor lib)

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.

NACK.

Integrating Kotlin for tests has been discussed and agreed on in bisq-network/events#28.

it was decided that kotlin needs to be removed.

Where is the discussion and the decision about getting rid of Kotlin you refer to?

@battleofwizards
Copy link
Contributor

battleofwizards commented Sep 10, 2019

Where is the discussion

https://bisq.slack.com/archives/C8MU98VSL/p1568055926265800

kotlin2

In short: apart from the event 28 there never seemed to be a wide consensus to introduce Kotlin.

@christophsturm
Copy link
Contributor Author

christophsturm commented Sep 10, 2019

Where is the discussion and the decision about getting rid of Kotlin you refer to?

@ManfredKarrer told me in a private chat that he wants it gone.

@battleofwizards
Copy link
Contributor

It would be nice to move forward with this.

@ripcurlx
Copy link
Contributor

@freimair Is your NACK for this PR still standing?

@freimair
Copy link
Contributor

freimair commented Sep 27, 2019

@ripcurlx, @ALL: yes.

We discussed the testing strategies and how to proceed with Kotlin in this dev-call. (TL;DR of the Kotlin-section of the dev-call: Only use Kotlin for tests, get some experience, see how it does. In months time, we evaluate again and see if we want Kotlin for tests or not and see if we want Kotlin for functional Bisq or not. If we maybe want Kotlin, there will be a proposal.)

There is hardly any reasoning on why the strategy needs reverting (no facts, no refs, just feelings), neither has there been a documented discussion with an agreement against the strategy agreed on. There are some voices against Kotlin, again no documented discussion, no facts, and no documented agreements.

If you do not agree with the strategy, please initiate a proper discussion with real facts, pros and cons and a documented agreement and I am happy to revoke my nack.

Why the trouble: If we start ignoring decisions we already made and making up new ones without a proper discussion, facts and agreements, we for one

  • are not really appealing for new developers,
  • we will not be able to get our >1000BTC per month project that stable as it should be and
  • an attacker might revoke important decisions without any discussion and maybe make an (her) attack possible.

This PR happens to be about Kotlin, but it is really just an example how it should not be. Btw. a dev-call has been held to address this topic. And please feel free to add interesting topics to the dev-call-collector, eg. if you want to discuss why we should get rid of Kotlin.

All in all, I do not see why we should spend time with discussing this over and over again. Eventually (according to the strategy agreed on), there comes the time where we decide whether to use Kotlin or not. Until then, I suggest focusing our resources where they are needed.

@ripcurlx
Copy link
Contributor

ripcurlx commented Dec 6, 2019

Closing this PR because of inactivity for more than 30 days and an open NACK.

@ripcurlx ripcurlx closed this Dec 6, 2019
@julianknutsen
Copy link
Contributor

Added my feedback to the open issue: https://github.com/bisq-network/bisq/pull/3235/files

Seems like the time has come to review the decision and decide if the future is Kotlin-less or Kotlin-enabled.

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.

5 participants