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 a few warnings #843

Merged
merged 6 commits into from Mar 12, 2020
Merged

Fix a few warnings #843

merged 6 commits into from Mar 12, 2020

Conversation

cb372
Copy link
Member

@cb372 cb372 commented Mar 12, 2020

Fix most of the compiler warnings.

Fix #827

@@ -25,6 +25,9 @@ import org.lyranthe.fs2_grpc.java_runtime.server.Fs2ServerCallHandler

object fs2Calls {

// TODO Support compression. It wasn't supported in fs2-grpc
// at the time this code was added (#477), but it is now.
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll tackle this in a separate PR

* reference it, so `_` gets turned into a fresh name e.g. `x$2`. The
* same thing happens even if you're not in a macro.)
*
* I'd say this is a bug in Scala. We work around it by manually adding
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to see if I can fix this easily in scala/scala.

Copy link
Member Author

Choose a reason for hiding this comment

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

@codecov
Copy link

codecov bot commented Mar 12, 2020

Codecov Report

Merging #843 into master will increase coverage by 0.2%.
The diff coverage is 10%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #843     +/-   ##
=========================================
+ Coverage   71.63%   71.84%   +0.2%     
=========================================
  Files          69       68      -1     
  Lines        1033     1030      -3     
  Branches       21       14      -7     
=========================================
  Hits          740      740             
+ Misses        293      290      -3
Impacted Files Coverage Δ
...main/scala/higherkindness/mu/server/fs2Calls.scala 75% <ø> (ø) ⬆️
.../main/scala/higherkindness/mu/http/implicits.scala 81.25% <ø> (ø) ⬆️
.../healthcheck/unary/handler/HealthServiceImpl.scala 100% <ø> (ø) ⬆️
...higherkindness/mu/rpc/internal/util/FileUtil.scala 0% <0%> (ø) ⬆️
.../higherkindness/mu/rpc/srcgen/util/AstOptics.scala 0% <0%> (ø) ⬆️
...ain/scala/higherkindness/mu/rpc/srcgen/Model.scala 18.18% <0%> (-1.82%) ⬇️
...herkindness/mu/rpc/kafka/KafkaManagementImpl.scala 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 67684e0...ebbd3e6. Read the comment docs.

Copy link
Member

@BenFradet BenFradet left a comment

Choose a reason for hiding this comment

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

👍 , it might be worth it to reestablish

scalacOptions --= Seq("-Xfuture", "-Xfatal-warnings"),
, wdyt?

args <- findAnnotation(d.mods, "http").collect({ case Apply(_, args) => args }).toList
d <- rpcDefs.collect { case x if findAnnotation(x.mods, "http").isDefined => x }
// TODO not sure what the following line is doing, as the result is not used. Is it needed?
_ <- findAnnotation(d.mods, "http").collect({ case Apply(_, args) => args }).toList
Copy link
Member

Choose a reason for hiding this comment

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

might be worth it to log an issue about it so we don't forget about it as easily, wdyt?

@cb372
Copy link
Member Author

cb372 commented Mar 12, 2020

@BenFradet For Scala 2.12, this PR gets us close to zero warnings, but not quite all the way there. We can't turn on -Xfatal-warnings just yet.

For Scala 2.13 there are lots of warnings about deprecation of JavaConverters, so we won't be able to turn on -Xfatal-warnings until we do something about that.

@cb372 cb372 merged commit 725e46f into master Mar 12, 2020
@cb372 cb372 deleted the fix-a-few-warnings branch March 12, 2020 14:41
@juanpedromoreno juanpedromoreno added the bug Something isn't working label Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix compiler warnings
3 participants