Skip to content
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

Rename start/endContextPropagation functions to asFlow/Source.. #26353

Merged

Conversation

Projects
None yet
5 participants
@skyluc
Copy link
Contributor

commented Feb 8, 2019

To match FlowWithContext.asFlow

Main changes in javadsl/Source.scala, javadsl/SourceWithContext.scala, javadsl/Source.scala and javadsl/SourceWithContext.scala. Plus update of the tests.

Flow.asFlowWithContext will be in a subsequent PR. It is more than a PR, as the function doesn't exist yet.

@akka-ci

This comment has been minimized.

Copy link
Collaborator

commented Feb 8, 2019

Thank you for your pull request! After a quick sanity check one of the team will reply with 'OK TO TEST' to kick off our automated validation on Jenkins. This compiles the project, runs the tests, and checks for things like binary compatibility and source code formatting. When two team members have also manually reviewed and (perhaps after asking for some amendments) accepted your contribution, it should be good to be merged.

For more details about our contributing process, check out CONTRIBUTING.md - and feel free to ask!

@johanandren

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

OK TO TEST

@akka-ci

This comment has been minimized.

Copy link
Collaborator

commented Feb 11, 2019

Test FAILed.

@johanandren

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

Needs renaming the corresponding operator docs as well to make the build happy-green

@patriknw patriknw requested a review from jrudolph Feb 12, 2019

@patriknw

This comment has been minimized.

Copy link
Member

commented Feb 12, 2019

The background for this change if I understood it correctly was that in Pipelines both startContextPropagation and endContextPropagation are not used together and then it is confusing to only endContextPropagation.

@patriknw
Copy link
Member

left a comment

LGTM after docs update

@skyluc

This comment has been minimized.

Copy link
Contributor Author

commented Feb 12, 2019

I updated the references to start/endContextPropagation.
I have not found how to test the doc, so I let jenkins do the check.
Travis is complaining about binary compatibility. I am not sure what should be done there.

@akka-ci

This comment has been minimized.

Copy link
Collaborator

commented Feb 12, 2019

Test FAILed.

@patriknw

This comment has been minimized.

Copy link
Member

commented Feb 12, 2019

complaining about binary compatibility

The whole "with context" is marked as ApiMayChange so add mima filters to akka-stream/src/main/mima-filters/2.5.20.backwards.excludes

(it will probably be 2.5.21 before this is merged so then it should be in 2.5.21.backwards.excludes)

@skyluc skyluc force-pushed the skyluc:issue/csp-1056-rename-flow-withcontext-transition branch from acd80f1 to 88111cb Feb 14, 2019

@akka-ci

This comment has been minimized.

Copy link
Collaborator

commented Feb 14, 2019

Test PASSed.

@jrudolph
Copy link
Member

left a comment

I think it's good for me. The new name is pretty explicit (while I found the old names more descriptive).

As we have decided and agreed on the names back and forth now quite a few times let's collect a few more votes this time so that we can call it final this time.

@skyluc skyluc force-pushed the skyluc:issue/csp-1056-rename-flow-withcontext-transition branch from 88111cb to 39c5d08 Feb 18, 2019

@akka-ci akka-ci added validating tested and removed tested labels Feb 18, 2019

@akka-ci akka-ci removed the validating label Feb 18, 2019

@akka-ci

This comment has been minimized.

Copy link
Collaborator

commented Feb 18, 2019

Test PASSed.

@johanandren
Copy link
Member

left a comment

LGTM but needs a rebase and type parameter switch now

@skyluc skyluc force-pushed the skyluc:issue/csp-1056-rename-flow-withcontext-transition branch from 39c5d08 to d16668e Feb 21, 2019

@akka-ci

This comment has been minimized.

Copy link
Collaborator

commented Feb 21, 2019

Test PASSed.

@patriknw

This comment has been minimized.

Copy link
Member

commented Mar 4, 2019

seems this rename has been given enough time for comments now, so merging

@patriknw patriknw merged commit 5a425c1 into akka:master Mar 4, 2019

3 checks passed

Jenkins PR Validation Test PASSed. 4639 tests run, 491 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
typesafe-cla-validator All users have signed the CLA
Details
@patriknw

This comment has been minimized.

Copy link
Member

commented Mar 4, 2019

Refs #26458

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.