build: update sbt and using sbt-tpolcat #57

ngbinh · 2020-12-29T06:52:35Z

Update sbt to 1.4.6
Update scala to 2.13.4
Start using
sbt-tpolcat
Un-deprecated EventStream.fromSeq: @raquo this is something I am not sure about. I am not sure what is the appropriate replacement for EventStream.fromSeq. WDYT?

src/main/scala/com/raquo/airstream/core/Observable.scala

src/main/scala/com/raquo/airstream/ownership/Owner.scala

src/test/scala/com/raquo/airstream/core/TransactionSpec.scala

.gitignore

src/main/scala/com/raquo/airstream/eventbus/WriteBus.scala

raquo · 2020-12-29T08:02:09Z

@ngbinh Thanks for the PR!

I actually already un-deprecated EventStream.fromSeq locally, just didn't have a chance to push it yet. Coming soon.

ngbinh · 2020-12-29T10:12:10Z

@raquo thanks for the reviews. I think address them all. Please take another look when you have time.

yurique · 2020-12-29T11:44:47Z

src/main/scala/com/raquo/airstream/core/Observable.scala

@@ -90,7 +91,7 @@ trait Observable[+A] {
  def debugLog(prefix: String = "event", when: A => Boolean = _ => true): Self[A] = {
    map(value => {
      if (when(value)) {
-        println(prefix + ": ", value.asInstanceOf[js.Any])
+        println(prefix + ": " + value.asInstanceOf[js.Any])


yurique · 2020-12-29T11:50:49Z

src/main/scala/com/raquo/airstream/core/Observer.scala

@@ -126,7 +128,7 @@ object Observer {
          if (onTryParam.isDefinedAt(nextValue)) {
            onTryParam(nextValue)
          } else {
-            nextValue.fold(err => AirstreamError.sendUnhandledError(err), identity)
+            nextValue.fold(err => AirstreamError.sendUnhandledError(err), _ => ())


I couldn't figure out why the success value case is dropped here. Maybe like this:

nextValue.failed.foreach { err => AirstreamError.sendUnhandledError(err) }

?

good catch!

hmm, Try,failed return a Failure in case of success. It would not as be clear as fold in this case IMHO.

Yeah, and because of that foreach will only kick in when the initial try is a failure. It's like with futures:

val f: Future[String] = ??? f.failed.foreach { case exception => ... }

With fold we're supposed to pass in a pure function, but we're dong side-effects and discarding the resulting Try.

Not insisting, but for me the intent was not clear initially :)

failed certainly looks nicer, but has a cost I'm not willing to pay in library code. With failed, on every incoming (non-error) event we would be initializing and discarding an exception: Failure(new UnsupportedOperationException("Success.failed")). And it seems that this will also generate a stack trace for that exception, which isn't super cheap.

If the intent of this line is unclear, we could add a comment to clarify, but what should it say? The code itself already says that it's reporting unhandled errors.

maybe like this then? :)

nextValue match { case Failure(err) => AirstreamError.sendUnhandledError(err) case _ => // nothing to do }

Last time I checked, maybe a couple years ago, pattern matches in scala.js were said to be not efficient. Things probably improved since then, but I don't have time to check the generated code atm.

I guess I'm just not sure what's trippy about this particular fold. The codebase is full of fold-s, this one having an empty case doesn't seem all that special to me.

yurique · 2020-12-29T12:00:39Z

fromSeq seems to be a useful part of the API. And with emitOnce it seems to be flexible enough.

But, I tried to see how and why exactly I use it — and couldn't find calls to it in my code anymore :) Only fromValue.

yurique · 2020-12-29T13:01:39Z

.travis.yml

@@ -9,8 +9,8 @@ jdk:
  - oraclejdk11

 scala:
-  - 2.12.10
-  - 2.13.1
+  - 2.12.11


latest seems to be 2.12.12

* Update sbt to 1.4.6 * Update scala to 2.13.4 * Start using [`sbt-tpolcat`](https://github.com/DavidGregory084/sbt-tpolecat)

…discard compiler option, and some other minor fixes.

raquo · 2020-12-30T01:43:27Z

Thanks @ngbinh I rebased and merged this into next-0.12, where the next release is shaping up.

raquo · 2021-01-05T04:17:29Z

@ngbinh Do you know how to disable the "unused imports / variables" scalac option only in Test? It's useful, but I found it's really annoying for tests as I like to temporarily comment out stuff when fixing tests, and then the rule fails compilation.

ngbinh · 2021-01-05T04:57:04Z

@raquo should be fairly easy. Do you want me to make a PR on top of #48 for that?

raquo · 2021-01-05T05:25:47Z

That would be great, thanks! I just have so much stuff to go through for 0.12 still.

On Mon., Jan. 4, 2021, 8:57 p.m. Binh Nguyen, ***@***.***> wrote: @raquo <https://github.com/raquo> should be fairly easy. Do you want me to make a PR on top of #48 <#48> for that? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#57 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAECBMHESMCA33VMZJSJONLSYKL23ANCNFSM4VM4BBFQ> .

--Nikita

…ns (#57)

ngbinh requested a review from raquo December 29, 2020 06:52