Skip to content

Conversation

@nickvergessen
Copy link
Member

The OC\Core\Command\Base is used in quite many apps
as it brings the base for different output methods
and allows autocomplete. Therefor this should be
exposed as OCP so apps are actually allowed to
benefit from it.

Signed-off-by: Joas Schilling [email protected]

The OC\Core\Command\Base is used in quite many apps
as it brings the base for different output methods
and allows autocomplete. Therefor this should be
exposed as OCP so apps are actually allowed to
benefit from it.

Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen nickvergessen added this to the Nextcloud 25 milestone May 11, 2022
@nickvergessen nickvergessen requested review from a team, CarlSchwan, ChristophWurst, skjnldsv and vanpertsch and removed request for a team May 11, 2022 07:14
/**
* @since 25.0.0
*/
abstract class ACommand extends Command implements CompletionAwareInterface {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we can expose Symphony/Comman in the public interface 😞

Copy link
Member Author

Choose a reason for hiding this comment

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

Well every app that providers an OCC command already uses it, either directly or by inheriting OC\Core\Command\Base, because only those can be registered:
https://github.com/nextcloud/3rdparty/blob/e0b9473487114accdba2e5a829ba763589ce5860/symfony/console/Application.php#L473

So I guess we just do it under that accepted risk

Copy link
Member

Choose a reason for hiding this comment

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

I would not because we don't get any independence from this.

Copy link
Member Author

@nickvergessen nickvergessen May 11, 2022

Choose a reason for hiding this comment

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

Well we would get a bit more exposure and I don't feel like starting to document private API or Symfony directly.

But I guess we then just don't have it nicely

@nickvergessen nickvergessen deleted the techdebt/noid/add-public-abstract-class-for-base-command branch May 11, 2022 11:38
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.

4 participants