Use value tuples in Streams #3282

vasily-kirichenko · 2018-01-20T15:10:12Z

No description provided.

general refactoring

Aaronontheweb · 2018-01-20T16:31:56Z

@vasily-kirichenko not sure why exactly, but it looks like the Mono compilation stage is having some trouble with these changes. I think we're running an older version of Mono 5.0* - should we upgrade the Docker image we're using for compilation there to see if that solves the issue? Windows compilation appears to be working just fine.

Aaronontheweb · 2018-01-20T16:32:34Z

.NET Core compilation on Linux works fine too.

vasily-kirichenko · 2018-01-20T19:51:27Z

I think we're running an older version of Mono 5.0* - should we upgrade the Docker image we're using for compilation there to see if that solves the issue?

I've never used Mono, and I've not found any mentions of Docker in build.fsx. How to do the upgrade?

Aaronontheweb · 2018-01-21T17:19:13Z

I've never used Mono, and I've not found any mentions of Docker in build.fsx. How to do the upgrade?

We run the build.fsx inside the Docker container built using the Dockerfile here for Mono builds: https://github.com/akkadotnet/dockerfiles

Those images eventually get pushed up here: https://hub.docker.com/r/akkadotnet/mono-base/

We use Docker for a few things in our CI system, mostly hosting the Mono environment and running Akka.Persistence integration tests, but that stuff mostly happens invisibly on our TeamCity agents and isn't exposed directly in the build scripts.

vasily-kirichenko · 2018-01-21T18:33:29Z

@Aaronontheweb akkadotnet/dockerfiles#7

marcpiechura · 2018-01-21T19:01:52Z

@vasily-kirichenko as @Aaronontheweb mentioned in the issue, this is quite a breaking change and should therefore target the v1.4 branch.

BTW, thanks a lot for taking this one 😄

vasily-kirichenko · 2018-01-21T19:18:33Z

@marcpiechura done

marcpiechura

Looks verg good overall, only a few small formatting remarks

marcpiechura · 2018-01-25T21:25:46Z

src/core/Akka.Streams.Tests/Actor/ActorPublisherSpec.cs

@@ -498,7 +497,8 @@ protected override bool Receive(object message)
                .With<Complete>(OnComplete)
                .With<CompleteThenStop>(OnCompleteThenStop)
                .With<Boom>(() => { throw new Exception("boom"); })
-                .With<ThreadName>(()=>_probe.Tell(Context.Props.Dispatcher /*Thread.CurrentThread.Name*/)) // TODO fix me when thread name is set by dispatcher
+                .With<ThreadName


Revert formatting and maybe put the comment in the line above.

marcpiechura · 2018-01-25T21:26:18Z

src/core/Akka.Persistence.TCK/PluginSpec.cs

-            Dispose(true);
-            GC.SuppressFinalize(this);
-        }
-


Are you sure that we can remove this part ?

Yes. The same code is in the base class's Dispose method.

marcpiechura · 2018-01-25T21:36:41Z

src/core/Akka.Streams.Tests/Dsl/RestartSpec.cs

@@ -141,15 +154,20 @@ public void A_restart_with_backoff_source_should_backoff_before_restart()
        }

        [Fact]
-        public void A_restart_with_backoff_source_should_reset_exponential_backoff_back_to_minimum_when_source_runs_for_at_least_minimum_backoff_without_completing()
+        public void
+            A_restart_with_backoff_source_should_reset_exponential_backoff_back_to_minimum_when_source_runs_for_at_least_minimum_backoff_without_completing()


Put it back in one line

marcpiechura · 2018-01-25T21:45:53Z

src/core/Akka.Streams.Tests/Dsl/RestartSpec.cs

+                    {
+                        "a",
+                        "b"
+                    });


Would prefer the single line and array syntax, but isn’t necessary

marcpiechura · 2018-01-25T21:46:34Z

src/core/Akka.Streams.Tests/Dsl/RestartSpec.cs

@@ -326,14 +344,13 @@ public void A_restart_with_backoff_sink_should_backoff_before_restart()
        }

        [Fact]
-        public void A_restart_with_backoff_sink_should_reset_exponential_backoff_back_to_minimum_when_source_runs_for_at_least_minimum_backoff_without_completing()
+        public void
+            A_restart_with_backoff_sink_should_reset_exponential_backoff_back_to_minimum_when_source_runs_for_at_least_minimum_backoff_without_completing()


Back in one line

marcpiechura · 2018-01-25T22:15:43Z

src/core/Akka.Streams/Implementation/ActorMaterializerImpl.cs

                    materializedValues.Add(atomic, materialized);
                }
                //else if (atomic is TlsModule)
                //{
                //})
                else if (atomic is GraphModule)
                {
-                    var graph = (GraphModule) atomic;
+                    var graph = (GraphModule)atomic;


Why not also use assignment in the if statement ?

marcpiechura · 2018-01-25T22:15:55Z

src/core/Akka.Streams/Implementation/ActorMaterializerImpl.cs

                    MaterializeGraph(graph, effectiveAttributes, materializedValues);
                }
                else if (atomic is GraphStageModule)
                {
-                    var stage = (GraphStageModule) atomic;
+                    var stage = (GraphStageModule)atomic;


# Conflicts: # src/core/Akka.Streams.Tests/Dsl/HubSpec.cs

no-brain auto refactoring

vasily-kirichenko · 2018-01-26T11:59:05Z

@marcpiechura all done.

Aaronontheweb · 2018-01-26T18:16:37Z

Going to pull the plug on our Mono builds, I think; been talking with @Horusiath about dropping support for it since it affects PRs like this one and our IObservable integration. Going to restore our .NET Core on Linux builds and use that as our canary for x-plat compatibility on POSIX operating systems.

marcpiechura · 2018-02-08T17:42:56Z

@Aaronontheweb any timeframe for removing mono? Would like to merge this one and rebase my work for Streams 2.5 on it

Aaronontheweb · 2018-06-14T12:08:52Z

I'll look at resolving the conflicts here and getting this pulled into v1.4 now that v1.4 has been updated with the latest sources.

Aaronontheweb · 2019-07-18T02:13:39Z

Need to give this another look for the next Akka.NET v1.4.0 beta

Aaronontheweb · 2019-09-06T21:41:04Z

Retargeting the dev branch here...

Aaronontheweb · 2019-09-06T22:11:19Z

The merge conflicts are a little nasty - I think I'll need to do this offline.

Aaronontheweb · 2019-10-24T15:00:16Z

closed via #3973

use value tuple instead of ref tuple in Keep.Both

4c85b19

vasily-kirichenko mentioned this pull request Jan 20, 2018

Use ValueTuple instead of Tuple #3276

Closed

vasily-kirichenko added 4 commits January 20, 2018 18:31

use value tuple in ProcessorModule

c2c4f18

general refactoring

use value tuple in UnzipWith

627191f

use value tuple in GetStageActorRef

97177e5

more value tuples

ada8d71

vasily-kirichenko added 4 commits January 20, 2018 20:21

more value tuples

b3eb41f

more value tuples

7455a19

more value tuples

21521dd

more value tuples

a7aa4ca

vasily-kirichenko changed the title ~~Use value tuple instead of ref tuple in Keep.Both~~ Use value tuples in Streams Jan 20, 2018

vasily-kirichenko added 2 commits January 20, 2018 22:15

more value tuples

bf5fdc9

more value tuples

e91f83d

vasily-kirichenko added 3 commits January 21, 2018 11:06

fix tests

5d5d40c

fix tests

b1f2a31

more value tuples

15a5b50

vasily-kirichenko changed the base branch from dev to v1.4 January 21, 2018 19:18

marcpiechura suggested changes Jan 25, 2018

View reviewed changes

Vasily Kirichenko added 2 commits January 26, 2018 12:37

Merge branch 'dev' into value-tuple-in-keep-both

e5d13ea

# Conflicts: # src/core/Akka.Streams.Tests/Dsl/HubSpec.cs

address code review

008ed0b

no-brain auto refactoring

marcpiechura approved these changes Jan 29, 2018

View reviewed changes

Danthar added this to the 1.4.0 milestone Apr 18, 2018

marcpiechura mentioned this pull request Jun 14, 2018

ValueTuple for all projects except for streams #3332

Closed

Aaronontheweb self-assigned this Jul 18, 2019

Aaronontheweb changed the base branch from v1.4 to dev September 6, 2019 21:40

Aaronontheweb mentioned this pull request Sep 10, 2019

Akka.Streams ValueTuple (cont'd) #3907

Closed

Aaronontheweb mentioned this pull request Oct 16, 2019

Moving to ValueTuples #3973

Merged

Aaronontheweb closed this Oct 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use value tuples in Streams #3282

Use value tuples in Streams #3282

vasily-kirichenko commented Jan 20, 2018

Aaronontheweb commented Jan 20, 2018

Aaronontheweb commented Jan 20, 2018

vasily-kirichenko commented Jan 20, 2018

Aaronontheweb commented Jan 21, 2018

vasily-kirichenko commented Jan 21, 2018

marcpiechura commented Jan 21, 2018

vasily-kirichenko commented Jan 21, 2018

marcpiechura left a comment

marcpiechura Jan 25, 2018

marcpiechura Jan 25, 2018

vasily-kirichenko Jan 26, 2018

marcpiechura Jan 25, 2018

marcpiechura Jan 25, 2018 •

edited

marcpiechura Jan 25, 2018

marcpiechura Jan 25, 2018

marcpiechura Jan 25, 2018

vasily-kirichenko commented Jan 26, 2018

Aaronontheweb commented Jan 26, 2018

marcpiechura commented Feb 8, 2018

Aaronontheweb commented Jun 14, 2018

Aaronontheweb commented Jul 18, 2019

Aaronontheweb commented Sep 6, 2019

Aaronontheweb commented Sep 6, 2019

Aaronontheweb commented Oct 24, 2019

Use value tuples in Streams #3282

Use value tuples in Streams #3282

Conversation

vasily-kirichenko commented Jan 20, 2018

Aaronontheweb commented Jan 20, 2018

Aaronontheweb commented Jan 20, 2018

vasily-kirichenko commented Jan 20, 2018

Aaronontheweb commented Jan 21, 2018

vasily-kirichenko commented Jan 21, 2018

marcpiechura commented Jan 21, 2018

vasily-kirichenko commented Jan 21, 2018

marcpiechura left a comment

Choose a reason for hiding this comment

marcpiechura Jan 25, 2018

Choose a reason for hiding this comment

marcpiechura Jan 25, 2018

Choose a reason for hiding this comment

vasily-kirichenko Jan 26, 2018

Choose a reason for hiding this comment

marcpiechura Jan 25, 2018

Choose a reason for hiding this comment

marcpiechura Jan 25, 2018 • edited

Choose a reason for hiding this comment

marcpiechura Jan 25, 2018

Choose a reason for hiding this comment

marcpiechura Jan 25, 2018

Choose a reason for hiding this comment

marcpiechura Jan 25, 2018

Choose a reason for hiding this comment

vasily-kirichenko commented Jan 26, 2018

Aaronontheweb commented Jan 26, 2018

marcpiechura commented Feb 8, 2018

Aaronontheweb commented Jun 14, 2018

Aaronontheweb commented Jul 18, 2019

Aaronontheweb commented Sep 6, 2019

Aaronontheweb commented Sep 6, 2019

Aaronontheweb commented Oct 24, 2019

marcpiechura Jan 25, 2018 •

edited