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

Flow.monitor signature is not nice, would be monitorMat #24812

Closed
ktoso opened this Issue Mar 29, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@ktoso
Member

ktoso commented Mar 29, 2018

Likely we can't fix it, but monitor signature is now:

def monitor[Mat2]()(combine: (Mat, FlowMonitor[Out]) ⇒ Mat2): ReprMat[Out, Mat2] =

which is a "Mat" style.

It should be:

def monitorMat[Mat2]()(combine: (Mat, FlowMonitor[Out]) ⇒ Mat2): ReprMat[Out, Mat2] =

and

def monitor(): ReprMat[FlowMonitor[Out]] =

Though I don't think we can fix it nowadays due to bin compat

@johanandren

This comment has been minimized.

Show comment
Hide comment
@johanandren

johanandren Mar 29, 2018

Member

I agree, would have been a bit nicer.

Member

johanandren commented Mar 29, 2018

I agree, would have been a bit nicer.

@ktoso ktoso added 1 - triaged and removed 0 - new labels Mar 29, 2018

@patriknw

This comment has been minimized.

Show comment
Hide comment
@patriknw

patriknw Mar 29, 2018

Member

Add, deprecate, remove (later)

Member

patriknw commented Mar 29, 2018

Add, deprecate, remove (later)

@ktoso

This comment has been minimized.

Show comment
Hide comment
@ktoso

ktoso Mar 30, 2018

Member

I was worried if the new signature would conflict with the existing one though due the multiple param list hm... maybe it'd work

Member

ktoso commented Mar 30, 2018

I was worried if the new signature would conflict with the existing one though due the multiple param list hm... maybe it'd work

@ktoso ktoso self-assigned this Apr 9, 2018

ktoso added a commit to ktoso/akka that referenced this issue Apr 9, 2018

@ktoso

This comment has been minimized.

Show comment
Hide comment
@ktoso

ktoso Apr 9, 2018

Member

Add, deprecate, remove (later)

Yeah so sadly it is as I feared and it does cause source incompatibility in Scala, Did a quick try about this

Member

ktoso commented Apr 9, 2018

Add, deprecate, remove (later)

Yeah so sadly it is as I feared and it does cause source incompatibility in Scala, Did a quick try about this

@ktoso ktoso added 3 - in progress and removed 1 - triaged labels Apr 9, 2018

ktoso added a commit to ktoso/akka that referenced this issue Apr 9, 2018

ktoso added a commit to ktoso/akka that referenced this issue Apr 20, 2018

ktoso added a commit to ktoso/akka that referenced this issue Apr 22, 2018

ktoso added a commit to ktoso/akka that referenced this issue Apr 23, 2018

ktoso added a commit to ktoso/akka that referenced this issue Sep 20, 2018

patriknw added a commit that referenced this issue Sep 25, 2018

+str #24812 fix signature of monitor()
* make monitor be a keep.both by default

patriknw added a commit that referenced this issue Sep 25, 2018

@patriknw patriknw added this to the 2.5.17 milestone Sep 25, 2018

@patriknw patriknw removed the discuss label Sep 25, 2018

@patriknw patriknw closed this Sep 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment