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

chore: Remove ApiMayChange on Projection and RES over gRPC #1170

Merged
merged 3 commits into from
Jun 18, 2024

Conversation

patriknw
Copy link
Member

No description provided.

@patriknw patriknw requested a review from johanandren June 13, 2024 11:06
@@ -556,7 +556,7 @@ private[projection] class R2dbcOffsetStore(
val seqNr = recordWithOffset.record.seqNr
val currentState = getState()

val duplicate = getState().isDuplicate(recordWithOffset.record)
val duplicate = currentState.isDuplicate(recordWithOffset.record)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unrelated, small cleanup that I had lying around

Copy link
Member

@octonato octonato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@johanandren johanandren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with some smaller stuff

@@ -592,7 +590,6 @@ object ConsumerFilter extends ExtensionId[ConsumerFilter] {

}

@ApiMayChange
class ConsumerFilter(system: ActorSystem[_]) extends Extension {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should maybe be final since not meant for user extension

@@ -83,7 +81,6 @@ object GrpcReadJournal {

}

@ApiMayChange
class GrpcReadJournal(delegate: scaladsl.GrpcReadJournal)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final since public and not meant for extension

Comment on lines 18 to 19
*/
@ApiMayChange
abstract class ReplicatedBehaviors[Command, Event, State] {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

* Not for user extension
*/
@DoNotInherit

Both language versions, we implement it and hand it to the user who only uses it

@@ -23,7 +22,6 @@ object Replica {

}

@ApiMayChange
trait Replica {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
trait Replica {
/**
* Not for user extension
*/
@DoNotInherit
trait Replica {

@@ -19,7 +18,6 @@ import akka.stream.scaladsl.FlowWithContext
/**
* Factory/function for creating the projection where offsets are kept track of for the replication streams
*/
@ApiMayChange
trait ReplicationProjectionProvider {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
trait ReplicationProjectionProvider {
@FunctionalInterface
trait ReplicationProjectionProvider {

@patriknw
Copy link
Member Author

thanks, incorporated feedback

@patriknw patriknw merged commit a33d732 into main Jun 18, 2024
22 checks passed
@patriknw patriknw deleted the wip-grpc-apimaychage-patriknw branch June 18, 2024 12:21
@patriknw patriknw added this to the 1.5.5 milestone Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants