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

[Order] ShipmentProcessor renaming #4315

Merged

Conversation

lchrusciel
Copy link
Member

Q A
Bug fix? yes
New feature? no
BC breaks? yes
Deprecations? no
Fixed tickets
License MIT
Doc PR

OrderShipmentFactory is not a factory for sure. I'v renamed it to ShipmentProcessor because of lack of better name. Any suggestion are welcome.

Update: Name has been changed to OrderShipmentProcessor

@pjedrzejewski pjedrzejewski added the Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). label Feb 26, 2016
@lchrusciel lchrusciel force-pushed the renaming-shipment-processor branch from b098b8d to fccb000 Compare February 26, 2016 09:12
@lchrusciel lchrusciel changed the title [Behat] OrderProcessor processing [Order] ShipmentProcessor renaming Feb 26, 2016
@@ -68,7 +68,7 @@ public function __construct(OrderShipmentFactoryInterface $shipmentFactory, Ship
*/
public function processOrderShipments(GenericEvent $event)
{
$this->shipmentFactory->createForOrder(
$this->shipmentFactory->processOrderShipment(
$this->getOrder($event)
Copy link
Contributor

Choose a reason for hiding this comment

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

This will fit one line :)

@lchrusciel lchrusciel changed the title [Order] ShipmentProcessor renaming [WIP] [Order] ShipmentProcessor renaming Feb 26, 2016
$this->shouldImplement(ShipmentProcessorInterface::class);
}

function it_creates_a_single_shipment_and_assigns_all_unit_units_to_it(
Copy link
Contributor

Choose a reason for hiding this comment

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

...all_unit_units... ?

@lchrusciel lchrusciel force-pushed the renaming-shipment-processor branch 2 times, most recently from 2277298 to 5815dee Compare February 26, 2016 10:44
@lchrusciel lchrusciel force-pushed the renaming-shipment-processor branch from 5815dee to a9279c3 Compare February 26, 2016 13:32
@michalmarcinkowski
Copy link
Contributor

👍

@lchrusciel lchrusciel changed the title [WIP] [Order] ShipmentProcessor renaming [Order] ShipmentProcessor renaming Feb 26, 2016
pjedrzejewski pushed a commit that referenced this pull request Mar 1, 2016
@pjedrzejewski pjedrzejewski merged commit a7585fc into Sylius:master Mar 1, 2016
@pjedrzejewski
Copy link
Member

Thanks Łukasz!

@lchrusciel lchrusciel deleted the renaming-shipment-processor branch March 1, 2016 14:09
@Niiko
Copy link

Niiko commented Mar 1, 2016

Hi,

For consistency, just need to rename the parameter too.

 <parameter key="sylius.order_processing.shipment_factory.class">Sylius\Component\Core\OrderProcessing\OrderShipmentProcessor</parameter>

into

 <parameter key="sylius.order_processing.shipment_processor.class">Sylius\Component\Core\OrderProcessing\OrderShipmentProcessor</parameter>

@lchrusciel
Copy link
Member Author

@Niiko good catch. I have missed it totally.

pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
…essor

[Order] ShipmentProcessor renaming
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.

4 participants