-
Notifications
You must be signed in to change notification settings - Fork 156
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
Sort stream imports for pekko-stream modules #933
Conversation
"org.reactivestreams." | ||
"io.netty." | ||
"org.scalatest." | ||
"org.slf4j." | ||
"com.typesafe." |
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 updated the conf.
Are you sorting this using Intellij? |
@mdedetrich I'm using the |
stream-tests/src/test/scala/org/apache/pekko/stream/scaladsl/FlowOnErrorCompleteSpec.scala
Outdated
Show resolved
Hide resolved
Nice work, this is looking good especially considering its not messing with the |
Not critical but it also would be good to squash the |
When merging to the main branch, all commits will be converted to a single commit, right?
|
LGTM |
So this is the whole "rebase and merge" vs "squash and merge" debate. Some people have different preferences, but this PR specifically is an exception because since its adding a format commit we want to add it So ideally for this PR you would have one commit that does all of the necessary changes i.e. 84fb5f8 and then another commit which applies Otherwise we ordinarily don't care too much about rebase + merge versus squash + merge, although if you are the kind of person that adds lots of minor commits then you should use squash + merge. |
@laglangyue I will do a rebase later, it was in this style to make review easier. |
764d307
to
f59a9ec
Compare
f59a9ec
to
ae53336
Compare
Once this get merged, will follow up a |
@mdedetrich @pjfanning I would like to have this merged first:) |
ae53336
to
858b94b
Compare
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
Let me merge with with another pr which ignores the commit. |
Motivation:
When I do code modification in pekko-stream related module, after a
sortImports
I have to revert all unrelated changes, which is very annoying.Result:
Sort the imports in
pekko-stream
andpekko-stream-tests
.The
sort-imports
will cause problem when encounter#xxx
and does not obey the#scala-fix:off/on
too.