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 introduction of Kotlin compilation to the build #3887

Merged
merged 3 commits into from
Jan 20, 2020

Conversation

cbeams
Copy link
Contributor

@cbeams cbeams commented Jan 10, 2020

This PR is the second in a series of PRs leading up to a larger refactoring effort. It should be merged immediately after PR #3886.

Note that I originally authored this commit without being aware of the exact same commit having been submitted by @christophsturm in PR #3235. I have since followed up on the discussion there, and even though that PR was closed due to @freimair's open NACK, I am submitting this change again, strongly suggesting that it get merged for the reasons detailed in the commit message below. There are a number of additional changes we need to make to speed up the build, and this would be one of them in any case. There is simply no rationale for leaving this change in place as it was originally made, as it drains valuable developer time every time they run a build. I did not measure the amount of time it sucks up, and it was obvious to me that I didn't need to. Every time I ran a build, I stared at the compileKotlin task sitting there eating up cycles and time.

Let's please be pragmatic here and just get rid of this, at least for now. Please note that this is coming from someone who likes Kotlin very much; it's just not appropriate to keep in place in the overly-broad way it was applied and without adding any value.

f4ae5d312 (Chris Beams, 5 weeks ago)
   Revert "Apply kotlin plugin and convert one unused class to kotlin"

   This reverts commit 26c053dae80006338cb3df2e1ec6618cdcf58ff8 because Kotlin
   compilation slows down the build, was applied too broadly to all modules
   instead of just the one that needed it, and most importantly because we
   never actually went ahead with converting anything of importance to Kotlin.
   The commit being reverted was basically a demo, converting a single test
   type to show what kind of difference it would make.

Please disregard the additional non-Kotlin related commits in this PR. They are part of PR #3886 and will disappear here when that PR is merged.

Problem: Editorconfig was configured to strip trailing whitespace in all
file types. This is generally a good thing, but when interactive
rebasing with Git and editing the contents of a commit, trailing
whitespace is significant on blank lines and must be preserved in order
for edits to be applied cleanly.

Solution: Update Editorconfig to exclude *.diff from the strip
whitespace rule, as interactive rebasing edits are done in a temporary
file with a '.diff' suffix.
In the refactorings that will follow, Structure101 Studio and the
Structure101 Workspace IDEA plugin will be used to visualize and help
resolve tangles in the codebase. We were already ignore Studio
configuration files which end in .java.hsb, and this commit ignores the
newer Workspace product's .java.hsw files as well.
This reverts commit 26c053d because
Kotlin compilation slows down the build, was applied too broadly to all
modules instead of just the one that needed it, and most importantly
because we never actually went ahead with converting anything of
importance to Kotlin. The commit being reverted was basically a demo,
converting a single test type to show what kind of difference it would
make.
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK

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 fa3fcb4 into bisq-network:master Jan 20, 2020
@freimair
Copy link
Contributor

fine with it. The evaluation time we agreed on back then ran out and nothing has been created on the Kotlin front. Revoking my NACK has been on my todo-list.

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