Skip to content

Conversation

CodeDrivenMitch
Copy link
Member

@CodeDrivenMitch CodeDrivenMitch commented Mar 17, 2022

Proposal for adding a method to the SagaLifecycle, based on this discuss question.

This PR will resolve #1125.

@CodeDrivenMitch CodeDrivenMitch linked an issue Mar 17, 2022 that may be closed by this pull request
@CodeDrivenMitch CodeDrivenMitch self-assigned this Mar 18, 2022
@CodeDrivenMitch CodeDrivenMitch requested review from gklijs and smcvb March 18, 2022 08:01
@smcvb smcvb added Priority 4: Would Lowest priority. Would-be-nice to include issues when time allows it. Type: Feature Use to signal an issue is completely new to the project. Status: In Progress Use to signal this issue is actively worked on. labels Mar 18, 2022
@smcvb smcvb added this to the Release 4.6.0 milestone Mar 18, 2022
Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

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

The proposal is nice, let's make an actual implementation out of this.
Be sure to:

  • Update the copyright notices
  • Provide a usable implementation
  • Add documentation to the methods that may be used by contributors. This includes protected methods for when a user constructs their own impl.
  • Add tests!

@smcvb smcvb changed the title Proposal: SagaLifecycle.associationValues(). #1125 [#1125] Introduce SagaLifecycle.associationValues() Mar 18, 2022
@CodeDrivenMitch
Copy link
Member Author

@smcvb What do you mean by "Provide a usable implementation". It is usable as it is here, it was that simple. The methods were already there, it just needed to be added to the SagaLifecycle interface.

I will tidy up and write some tests to make it a good PR.

@CodeDrivenMitch CodeDrivenMitch requested a review from smcvb March 21, 2022 08:24
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@CodeDrivenMitch CodeDrivenMitch requested a review from a user March 21, 2022 08:44
@smcvb
Copy link
Member

smcvb commented Mar 21, 2022

@smcvb What do you mean by "Provide a usable implementation". It is usable as it is here, it was that simple. The methods were already there, it just needed to be added to the SagaLifecycle interface.

I will tidy up and write some tests to make it a good PR.

You got me here. This shows I don't know all code from Framework on top of my mind.

Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

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

My concerns have been addressed, hence approving.

@CodeDrivenMitch CodeDrivenMitch merged commit 8ea8532 into master Mar 22, 2022
@smcvb smcvb deleted the feature/1125 branch March 22, 2022 15:19
@smcvb smcvb added Status: Resolved Use to signal that work on this issue is done. and removed Status: In Progress Use to signal this issue is actively worked on. labels Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority 4: Would Lowest priority. Would-be-nice to include issues when time allows it. Status: Resolved Use to signal that work on this issue is done. Type: Feature Use to signal an issue is completely new to the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accessing Saga Association Values
2 participants