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

neo-sbt-scalafmt and making LoggedReporter extensible #413

Merged
merged 3 commits into from
Sep 25, 2017

Conversation

eed3si9n
Copy link
Member

I have two relatively minor changes here.

First is the adoption of neo-sbt-scalafmt. Without it I can't use emulated monorepo because all other modules are using neo-sbt-scalafmt.

Second, I am turning all the fields in LoggedReporter lazy. Without the change I get NPE when extending ManagedLoggedReporter for LSP. I think this is because ctor calls reset at line 107, and I am overriding it in the subclass, so it could get called before all fields are initialized. (Initialization still confuses me.) No other observable changes are expected.

@dwijnand
Copy link
Member

You'll have to tweak bin/run-ci.sh if you want to switch scalafmt plugin: https://github.com/sbt/zinc/pull/392/files#diff-4a4612d6cdda9aac96abca1deae9ec85R11.

Also the reason we're not already using neo-sbt-scalafmt is because of the CI issues I was seeing in #392.

@eed3si9n
Copy link
Member Author

@dwijnand Seems like Drone passed.

@jvican
Copy link
Member

jvican commented Sep 25, 2017

Can you reorder the commits in such a way that all commits pass CI?

I get NPE when I extend ManagedLoggedReporter if I don't do this.
@eed3si9n
Copy link
Member Author

@jvican Done.

Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

@dwijnand Seems like Drone passed.

Sweet. LGTM then!

Copy link
Member

@jvican jvican left a comment

Choose a reason for hiding this comment

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

Happy that we use neo-scalafmt now.

@jvican jvican merged commit 30c9bb4 into sbt:1.0.x Sep 25, 2017
@typesafe-tools
Copy link

The validator has checked the following projects against Scala 2.12,
tested using dbuild, projects built on top of each other.

Project Reference Commit
sbt 1.x sbt/sbt@4b8d31a
zinc pull/413/head 264ffc5
io 1.x sbt/io@62004b2
librarymanagement 1.x sbt/librarymanagement@293666f
util 1.x sbt/util@be73dc0
website 1.x

❌ The result is: FAILED
(restart)

@dwijnand dwijnand added this to the 1.0.2 milestone Oct 2, 2017
dwijnand added a commit to dwijnand/zinc that referenced this pull request Nov 23, 2017
* 1.0.x: (28 commits)
  Split compiler bridge tests to another subproject
  Implement compiler bridge for 2.13.0-M2
  Add yourkit acknoledgement in the README
  "sbt '++ 2.13.0-M2!' compile" does not work with sbt 1.0.0
  Add header to cached hashing spec
  Add headers to missing files
  Fix sbt#332: Add sbt-header back to the build
  Update sbt-scalafmt to 1.12
  Make classpath hashing more lightweight
  Fix sbt#442: Name hash of value class should include underlying type
  source-dependencies/value-class-underlying: fix test
  Ignore null in generic lambda tparams
  Improve and make scripted parallel
  Fix sbt#436: Remove annoying log4j scripted exception
  Fix sbt#127: Use `unexpanded` name instead of `name`
  Add pending test case for issue/127
  source-dependencies / patMat-scope workaround
  Fixes undercompilation on inheritance on same source
  Add real reproduction case for sbt#417
  Add trait-trait-212 for Scala 2.12.3
  ...

 Conflicts:
	internal/zinc-apiinfo/src/main/scala/sbt/internal/inc/ClassToAPI.scala
	project/build.properties
	zinc/src/main/scala/sbt/internal/inc/MixedAnalyzingCompiler.scala

The ClassToAPI conflict is due to:
* sbt#393 (a 1.x PR), conflicting with
* sbt#446 (a 1.0.x PR).

The build.properties conflict is due to different PRs bumping
sbt.version from 1.0.0 to 1.0.2 to 1.0.3. (sbt#413, sbt#418, sbt#453).

The MixedAnalyzingCompiler conflict is due to:
* sbt#427 (a 1.x PR), conflicting with
* sbt#452 (a 1.0.x PR).
@dwijnand dwijnand mentioned this pull request Nov 23, 2017
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