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

New plugin commands #63

Merged
merged 8 commits into from
Feb 28, 2024
Merged

New plugin commands #63

merged 8 commits into from
Feb 28, 2024

Conversation

afbora
Copy link
Member

@afbora afbora commented Feb 23, 2024

kirby plugin:install getkirby/kql
kirby plugin:remove getkirby/kql
kirby plugin:upgrade getkirby/kql

@afbora afbora added the type: feature 🎉 Adds a feature (requests → feedback.getkirby.com) label Feb 23, 2024
@afbora afbora added this to the 1.4.0 milestone Feb 23, 2024
@afbora afbora requested a review from a team February 23, 2024 07:53
@afbora afbora self-assigned this Feb 23, 2024
@afbora afbora force-pushed the feature/plugin branch 4 times, most recently from 7af5a45 to 8aea2ae Compare February 26, 2024 08:59
@afbora afbora force-pushed the feature/plugin branch 2 times, most recently from 92fc7a1 to ba79540 Compare February 27, 2024 11:56
@bastianallgeier
Copy link
Member

bastianallgeier commented Feb 27, 2024

I wonder if we should use the existing main groups instead of plugin:

kirby install:plugin getkirby/kql
kirby upgrade:plugin getkirby/kql
kirby remove:plugin getkirby/kql

Especially since there already is make:plugin.

We broke this for the uuid commands. But maybe we could change that in the future. I think we really just need to commit to one standard. @distantnative @lukasbestle what are your thoughts on this?

@lukasbestle
Copy link
Member

I agree it would be good to be consistent.

I don't have a strong preference about the order. Namespacing by type (like plugin:install) fits well semantically, but namespacing by command (like install:plugin) works better with commands like make (we have many of them and most types will only ever have a make command.

@afbora
Copy link
Member Author

afbora commented Feb 28, 2024

install:*, remove:*, upgrade:* syntax make sense but I still favor of plugin:* syntax. I'm thinking to create plugin command about versions. We can use plugin:* syntax for all plugin related commands. For example: kirby plugin:check and kirby plugin:check --all. This commands checks plugin(s) versions.

@bastianallgeier
Copy link
Member

I agree that the plugin namespace is a bit of an extra case here. I think there will be a lot of additional useful commands for plugins.

Maybe in this case we could think about adding aliases for install, remove and upgrade? On the other hand, aliases could also create even more confusion. Let's maybe keep the plugin namespace as default for now. We can still add aliases later if needed.

@afbora
Copy link
Member Author

afbora commented Feb 28, 2024

Great! Do you want to me change command names? uptate to upgrade and delete to remove.

@afbora afbora force-pushed the feature/plugin branch 3 times, most recently from 28d4d1d to a5e4991 Compare February 28, 2024 10:10
@afbora
Copy link
Member Author

afbora commented Feb 28, 2024

@bastianallgeier Ready to review and need your last touches 🙂

@bastianallgeier
Copy link
Member

I made a first few small changes and had a few thoughts:

plugin:install

I think we need to check for the plugin type in the composer json. So far I can basically install any Github repo and it does not really need to be a proper Kirby plugin. I think it would be good to use this as a sanity check.

plugin:remove

If a plugin does not have an index.php and does not call Kirby::plugin(), it cannot be removed.

@afbora
Copy link
Member Author

afbora commented Feb 28, 2024

@bastianallgeier If anonymous plugin installed (for ex: /site/plugins/hello-world), you need to call kirby plugin:remove plugins/hello-world. Anonymous plugins registry name is plugins/DIRNAME.

Copy link
Member

@bastianallgeier bastianallgeier left a comment

Choose a reason for hiding this comment

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

I'm happy with the current version. We can always make changes later if needed.

@bastianallgeier bastianallgeier merged commit 956ae92 into develop Feb 28, 2024
10 checks passed
@bastianallgeier bastianallgeier deleted the feature/plugin branch February 28, 2024 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature 🎉 Adds a feature (requests → feedback.getkirby.com)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants