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

[!!!][TASK] Drop additional namespace segment for the Tea model #1025

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

oliverklee
Copy link
Contributor

@oliverklee oliverklee commented Nov 27, 2023

The Product namespace segment in the domain model namespace TTN\Tea\Domain\Model currently serves no purpose and only adds confusion. So let's simplify the extension structure accordingly.

(I intended to use this to demonstrate DDD contexts, but never built enough models in the Tea extension for this to actually make sense.)

Fixes #1008

@coveralls
Copy link

Pull Request Test Coverage Report for Build 7006769465

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 7005572809: 0.0%
Covered Lines: 46
Relevant Lines: 46

💛 - Coveralls

Copy link
Contributor

@DanielSiepmann DanielSiepmann left a comment

Choose a reason for hiding this comment

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

I'd only question whether we want a breaking change right now.
Should we plan breaking changes together with major version releases?

Tests/Unit/Domain/Repository/TeaRepositoryTest.php Outdated Show resolved Hide resolved
Tests/Unit/Controller/FrontEndEditorControllerTest.php Outdated Show resolved Hide resolved
@oliverklee
Copy link
Contributor Author

Should we plan breaking changes together with major version releases?

Yes, makes sense. I've moved this to the 4.0 milestone now.

@oliverklee oliverklee marked this pull request as draft November 27, 2023 15:23
@oliverklee oliverklee force-pushed the task/drop-product-namespace branch 2 times, most recently from fc7a406 to d9576f8 Compare November 29, 2023 11:49
@oliverklee oliverklee force-pushed the task/drop-product-namespace branch 2 times, most recently from 7f6b8f8 to 58baae1 Compare December 19, 2023 08:58
@oliverklee oliverklee force-pushed the task/drop-product-namespace branch from 58baae1 to a837493 Compare January 2, 2024 22:29
@oliverklee oliverklee force-pushed the task/drop-product-namespace branch from a837493 to 585104b Compare January 8, 2024 21:55
@oliverklee oliverklee marked this pull request as ready for review January 8, 2024 21:56
@oliverklee oliverklee force-pushed the task/drop-product-namespace branch from 585104b to 90db6d8 Compare January 8, 2024 21:58
@oliverklee oliverklee force-pushed the task/drop-product-namespace branch from 90db6d8 to 8d4d45a Compare January 16, 2024 10:55
@oliverklee
Copy link
Contributor Author

@DanielSiepmann @lukaszuznanski ping for a review - thanks!

Copy link
Contributor

@lukaszuznanski lukaszuznanski left a comment

Choose a reason for hiding this comment

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

Should we add any migration guide?

@oliverklee
Copy link
Contributor Author

Should we add any migration guide?

I don't think this would we worth the effort in this case: Any data is test data anyway, and instead of migrating data, I'd prefer to have a script/command create the test data: #1120

The `Product` namespace segment in the domain model namespace
`TTN\Tea\Domain\Model` currently serves no purpose and only adds
confusion. So let's simplify the extension structure accordingly.

(I intended to use this to demonstrate DDD contexts, but never
built enough models in the Tea extension for this to actually
make sense.)

Fixes #1008
@oliverklee oliverklee force-pushed the task/drop-product-namespace branch from 8d4d45a to 572cd62 Compare January 16, 2024 13:52
@DanielSiepmann
Copy link
Contributor

Data is not affected anyway, so not a showcase for how to add UpgradeWizards anyway. This would only be an opportunity to provide a rector rule. But I also won't think we should do that right now.

@oliverklee oliverklee merged commit eeda862 into main Jan 16, 2024
51 checks passed
@oliverklee oliverklee deleted the task/drop-product-namespace branch January 16, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop the additional "Product" namespace from the models
4 participants