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

Shop billing data as resource #315

Merged
merged 8 commits into from
Jun 22, 2021

Conversation

SirDomin
Copy link
Contributor

No description provided.

@SirDomin SirDomin added the Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). label Jun 16, 2021
@SirDomin SirDomin requested a review from a team as a code owner June 16, 2021 16:30
src/Factory/ShopBillingDataFactory.php Outdated Show resolved Hide resolved
src/Entity/ShopBillingData.php Outdated Show resolved Hide resolved
src/Entity/ShopBillingDataInterface.php Show resolved Hide resolved
src/Entity/ShopBillingDataInterface.php Outdated Show resolved Hide resolved
src/Resources/config/services/generators.xml Outdated Show resolved Hide resolved
UPGRADE.md Show resolved Hide resolved
src/Entity/ShopBillingDataInterface.php Show resolved Hide resolved
src/Entity/ShopBillingDataInterface.php Show resolved Hide resolved
spec/Entity/ShopBillingDataSpec.php Outdated Show resolved Hide resolved
@@ -117,7 +123,7 @@ private function getToAddress(?ChannelShopBillingData $channelShopBillingData):
return null;
}

return new ShopBillingData(
return $this->shopBillingDataFactory->createWithData(
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a method in factory called createWithChannelShopBillingData ? 😉
See: https://github.com/Sylius/RefundPlugin/pull/313/files#diff-dc7a9ca1a71645aa3eb35cf2d342ab0543241c3b0173c6fe7d6ddf42a641be35R65

It won't block a merge imo - just a suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i would leave it as it is now

@SirDomin SirDomin force-pushed the shop-billing-data-as-resource branch 3 times, most recently from 7d30efa to 7d987b5 Compare June 17, 2021 07:48
@SirDomin SirDomin force-pushed the shop-billing-data-as-resource branch from 7d987b5 to d1a6869 Compare June 17, 2021 07:52
Copy link
Member

@Zales0123 Zales0123 left a comment

Choose a reason for hiding this comment

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

Removal of tests/Application/var/.gitignore is not relevant in the scope of this PR 🖖

src/Entity/ShopBillingData.php Outdated Show resolved Hide resolved
src/Entity/ShopBillingData.php Show resolved Hide resolved
src/Resources/config/app/config.yml Show resolved Hide resolved
src/Resources/config/services/generators.xml Outdated Show resolved Hide resolved
pamil
pamil previously requested changes Jun 17, 2021
src/Entity/ShopBillingDataInterface.php Outdated Show resolved Hide resolved
src/Entity/ShopBillingDataInterface.php Show resolved Hide resolved
phpstan.neon Outdated Show resolved Hide resolved
src/Entity/ShopBillingDataInterface.php Outdated Show resolved Hide resolved
src/Resources/config/services/generators.xml Outdated Show resolved Hide resolved
@SirDomin SirDomin force-pushed the shop-billing-data-as-resource branch 8 times, most recently from 42e35b0 to e08c3fb Compare June 18, 2021 09:20
Copy link
Member

@Zales0123 Zales0123 left a comment

Choose a reason for hiding this comment

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

tests/Application/var/.gitignore is still removed and it should not be 😄

UPGRADE.md Show resolved Hide resolved
src/Factory/ShopBillingDataFactory.php Outdated Show resolved Hide resolved
src/Resources/config/app/config.yml Show resolved Hide resolved
@SirDomin SirDomin force-pushed the shop-billing-data-as-resource branch 3 times, most recently from a034208 to 29935c5 Compare June 18, 2021 11:26
@SirDomin SirDomin force-pushed the shop-billing-data-as-resource branch 4 times, most recently from e18a91c to d9d84fa Compare June 18, 2021 12:10
@SirDomin SirDomin force-pushed the shop-billing-data-as-resource branch from d9d84fa to 1d4e338 Compare June 18, 2021 12:13
spec/Factory/ShopBillingDataFactorySpec.php Outdated Show resolved Hide resolved
src/Factory/ShopBillingDataFactory.php Outdated Show resolved Hide resolved
src/Factory/ShopBillingDataFactoryInterface.php Outdated Show resolved Hide resolved
src/Resources/config/app/config.yml Show resolved Hide resolved
@SirDomin SirDomin force-pushed the shop-billing-data-as-resource branch from dc0c514 to 6e24290 Compare June 21, 2021 14:34
@SirDomin SirDomin force-pushed the shop-billing-data-as-resource branch from 6e24290 to 71f26d2 Compare June 21, 2021 14:38
@Zales0123 Zales0123 dismissed pamil’s stale review June 22, 2021 07:26

Fixed provided 🖖

@Zales0123 Zales0123 merged commit 07993fb into Sylius:master Jun 22, 2021
@Zales0123
Copy link
Member

Thank you, @SirDomin! 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants