FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Upgrade to sbt-1.0.4 by xerial · Pull Request #443 · msgpack/msgpack-java · GitHub

/ msgpack-java Public

Upgrade to sbt-1.0.4 - #443

Merged
xerial merged 9 commits into
developfrom
sbt104
Nov 29, 2017
ConversationCommits (9)ChecksFiles changed
Merged

Upgrade to sbt-1.0.4 #443
xerial merged 9 commits into
developfrom
sbt104

Conversation

xerial commented Nov 29, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

For better Java9 support.

  • Migrate to sbt 1.0.4, which has better Java9 build support
  • Upgraded sbt-plugins for 1.0.4
  • Introduced scalafmt for Scala code formatting
  • Removed java7 test on Travis, but msgpack-java is still built for Java 7 (-target 1.7)
    • For Java7 testing, we should create a pre-built testing program and run it on JDK7

xerial changed the title Upgrade to sbt-1.0.4 [WIP] Upgrade to sbt-1.0.4 Nov 29, 2017

xerial commented Nov 29, 2017

Copy link
Copy Markdown
Member Author

Found one performance regression:

[info] - should be faster than msgpack-v6 read value *** FAILED ***
[info]   0.0027447835408163264 was not less than or equal to 0.0022912134897959185 (MessageUnpackerTest.scala:595)

xerial commented Nov 29, 2017

Copy link
Copy Markdown
Member Author

The performance regression never occurs in my local machine. It seems a problem of running test code inside a container of Travis. That test should be more lenient (e.g., just show warning)

xerial changed the title [WIP] Upgrade to sbt-1.0.4 Upgrade to sbt-1.0.4 Nov 29, 2017

xerial commented Nov 29, 2017

Copy link
Copy Markdown
Member Author

@komamitsu Could you check this PR? (cc: @xuwei-k)
This fixes recent build failures on Travis, and general updates of sbt plugins to the latest version.
FYI. sbt-1.0.4 supports Scala 2.12 and Java9. So we no longer need to use Scala 2.10.

xerial requested a review from komamitsu November 29, 2017 07:29
xuwei-k reviewed Nov 29, 2017
View reviewed changes
Comment thread README.md
> findbugs # Produce findbugs report in target/findbugs
> jacoco:cover # Report the code coverage of tests to target/jacoco folder
> jcheckStyle # Run check style
> scalafmt # Reformat Scala codes

xuwei-k Nov 29, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

https://github.com/lucidsoftware/neo-sbt-scalafmt/blob/1.14/README.md#usage

scalafmt # format compile sources
test:scalafmt # format test sources
sbt:scalafmt # format .sbt source

The scalafmt command format only compile sources

xuwei-k Nov 29, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

https://github.com/msgpack/msgpack-java/pull/443/files#r153711477

xuwei-k reviewed Nov 29, 2017
View reviewed changes
Comment thread .travis.yml
- env: PROJECT=java8
jdk: oraclejdk8
script:
- ./sbt test

xuwei-k Nov 29, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

add sbt ";scalafmt::test ;test:scalafmt::test ;sbt:scalafmt::test" ?

https://github.com/lucidsoftware/neo-sbt-scalafmt/blob/1.14/README.md#usage
https://github.com/sbt/sbt/blob/v1.0.4/.travis.yml#L20

To ensure everything is formatted, and fail if it is not (e.g. as a CI step),

> scalafmt::test # check compile sources
> test:scalafmt::test # check test sources
> sbt:scalafmt::test # check .sbt sources

xuwei-k Nov 29, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ah, Maybe I was wrong. You added addSbtPlugin("com.geirsson" % "sbt-scalafmt" % "1.3.0"), not neo-sbt-scalafmt 🙇

xerial Nov 29, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Right. sbt-scalafmt reformats everything including main, test, *.sbt, etc.

komamitsu approved these changes Nov 29, 2017
View reviewed changes

komamitsu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Totally LGTM

But can you check the feedbacks from @xuwei-k on scalafmt (Sorry I'm not a Scala guy... 😁 )

xerial commented Nov 29, 2017

Copy link
Copy Markdown
Member Author

Thanks for checking! @komamitsu @xuwei-k

xerial merged commit eddb753 into develop Nov 29, 2017
xerial deleted the sbt104 branch November 29, 2017 08:00
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

komamitsu komamitsu approved these changes
+1 more reviewer
xuwei-k xuwei-k left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL