Skip to content

AggregateRoot OnDSL and DefaultApplyStrategy always go together #982

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

Closed
mostlyobvious opened this issue Jan 9, 2021 · 2 comments
Closed
Milestone

Comments

@mostlyobvious
Copy link
Member

Yet it is possible to have AggregateRoot.with_strategy(->{ DefaultApplyStrategy.new }) and no OnDSL. This clearly signals needed change.

# include AggregateRoot                                                 | on_, apply_, value.to_s, strict: true
# include AggregateRoot.with(strategy: custom)                          | _  ,      _, value.to_s, _
# include AggregateRoot.with(event_type_resolver: custom)               | _  , apply_, custom,     strict: true  | XXX on_methods missing
# include AggregateRoot.with(strategy: ->{ DefaultApplyStrategy.new })  | _  , apply_, value.to_s, strict: true  | XXX on_methods missing

PS. event_type_resolver refers to #969

@mostlyobvious mostlyobvious added this to the v3.0.0 milestone Jan 9, 2021
@fidel
Copy link
Contributor

fidel commented Jan 3, 2023

I think it's not clear which direction is desired 🤷‍♂️

@fidel
Copy link
Contributor

fidel commented Jan 9, 2023

DefaultApplyStrategy relies on on_methods, so it shouldn't be possible to it separately.

lukaszreszke added a commit that referenced this issue Feb 8, 2023
DefaultApplyStrategy relies on OnDSL. Therefore they always have to go
together.

https: //github.com//issues/982

Co-authored-by: Paweł Pacana <[email protected]>
Co-authored-by: Szymon Fiedler <[email protected]>
lukaszreszke added a commit that referenced this issue Feb 10, 2023
DefaultApplyStrategy relies on OnDSL. Therefore they always have to go
together.

https: //github.com//issues/982

Co-authored-by: Paweł Pacana <[email protected]>
Co-authored-by: Szymon Fiedler <[email protected]>
fidel added a commit that referenced this issue Mar 7, 2023
DefaultApplyStrategy relies on OnDSL. Therefore they always have to go
together.

https: //github.com//issues/982

Co-authored-by: Paweł Pacana <[email protected]>
Co-authored-by: Szymon Fiedler <[email protected]>
mostlyobvious added a commit that referenced this issue Sep 29, 2023
DefaultApplyStrategy relies on OnDSL. Therefore they always have to go
together.

https: //github.com//issues/982

Co-authored-by: Paweł Pacana <[email protected]>
Co-authored-by: Szymon Fiedler <[email protected]>
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

No branches or pull requests

3 participants