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.

Sign up for GitHub

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

Jump to bottom

Fix EZP-26393: Missing options for ezpublish_legacy.kernel_handler.cli #70

Merged
merged 1 commit into from
Oct 13, 2016
Merged

Fix EZP-26393: Missing options for ezpublish_legacy.kernel_handler.cli #70

merged 1 commit into from
Oct 13, 2016

Conversation

lolautruche
Copy link
Contributor

@lolautruche lolautruche commented Oct 10, 2016 •
edited
Loading

https://jira.ez.no/browse/EZP-26393

This patch enforces use-modules to true, so that operations like content/publish can be used from legacy scripts:

eZOperationHandler::execute('content', 'publish', [
    'object_id' => $objectId, 
    'version' => $version,
]);

@lolautruche
Fix EZP-26393: Missing options for ezpublish_legacy.kernel_handler.cli
368f33a 
> https://jira.ez.no/browse/EZP-26393

This patch enforces `use-modules` to `true`, so that operations like `content/publish` can be used from legacy scripts:

```php
eZOperationHandler::execute('content', 'publish', ['object_id' => $objectId, 'version' => $version]);
```
@lolautruche
Copy link
Contributor Author

Ping @andrerom @bdunogier @yannickroger @glye

@emodric
Copy link
Collaborator

emodric commented Oct 10, 2016

+1

Why not include use-extensions too?

@lolautruche
Copy link
Contributor Author

Why not include use-extensions too?

Because it's true by default 😉

@emodric
Copy link
Collaborator

emodric commented Oct 10, 2016

Because it's true by default 😉

Well, damn! :)

@glye
Copy link
Member

glye commented Oct 10, 2016

Not knowing this stuff, is this just something that has been forgotten, or are there any downsides/tradeoffs?
The reason for not changing the use-modules default value instead is... BC?

@lolautruche
Copy link
Contributor Author

Yes, when implementing the CLIHandler, I actually forgot an important usecase: operations, which are bound on modules.

I remember wondering why would we need modules in a script... Stupid me.

@lolautruche
Copy link
Contributor Author

There are no tradeoffs AFAIK

glye
glye approved these changes Oct 10, 2016
View reviewed changes
Copy link
Member

@glye glye left a comment

Choose a reason for hiding this comment

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

Thanks, +1

@bdunogier
Copy link
Member

+1

@lolautruche
Copy link
Contributor Author

If everyone approves, maybe this can be merged? :-)

@lolautruche
Copy link
Contributor Author

Ping ? :)

@bdunogier bdunogier merged commit 6627491 into ezsystems:master Oct 13, 2016
@blankse blankse mentioned this pull request Dec 7, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emodric emodric emodric approved these changes

@andrerom andrerom andrerom approved these changes

@glye glye glye approved these changes

@bdunogier bdunogier bdunogier approved these changes

Assignees

@bdunogier bdunogier

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lolautruche @emodric @glye @bdunogier @andrerom