-
Notifications
You must be signed in to change notification settings - Fork 5
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
Migrate to sbt-http4s-org
plugin
#97
Conversation
ba3a406
to
6bb0ae0
Compare
d9fd490
to
977e0be
Compare
977e0be
to
e53ca60
Compare
@armanbilge @valencik Sorry for that diff 🤷🏻 |
// disable sbt-header plugin until we are not aligned on the license | ||
ThisBuild / headerCheckAll := Nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
build.sbt
Outdated
@@ -1,13 +1,13 @@ | |||
import explicitdeps.ExplicitDepsPlugin.autoImport.moduleFilterRemoveValue | |||
|
|||
ThisBuild / tlBaseVersion := "0.0" // your current series x.y |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm willing to set it to 0.1
. Any objections?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Confirmed scalafix/scalafmt setup are the same as http4s.
I noticed a couple things we could perhaps tidy up, but we could also do so in a followup PR to reduce noise here.
One thing we should do is add the commit where the scalafmt is first run to a .git-blame-ignore-revs
file like Scala Steward does for us.
So a new .git-blame-ignore-revs
file with the content:
# #97 Introduce sbt-http4s-org plugin
b92d1dbfcc29d3afcca0d2c143528f07b949dcd3
for { | ||
paramsAndUnparsed <- GeneratorParams.fromStringCollectUnrecognized(params) | ||
params = paramsAndUnparsed._1 | ||
unparsed = paramsAndUnparsed._2 | ||
// unparsed = paramsAndUnparsed._2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just remove this if it's unused?
val serviceCall = s"serviceImpl.${method.name}" | ||
val eval = if (method.isServerStreaming) s"$Stream.eval(mkCtx(m))" else "mkCtx(m)" | ||
// val serviceCall = s"serviceImpl.${method.name}" | ||
// val eval = if (method.isServerStreaming) s"$Stream.eval(mkCtx(m))" else "mkCtx(m)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly. Or perhaps we want to leave that kind of tidying for another PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I presumed there could be many other refactorings, so I just put this under comments.
Maybe Arman would have something to add. But we already have a quorum of approvers, so merging this. |
No description provided.