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

Allow multiple commands for the same plugin #75

Closed
gentlementlegen opened this issue Jul 16, 2024 · 9 comments · Fixed by #77
Closed

Allow multiple commands for the same plugin #75

gentlementlegen opened this issue Jul 16, 2024 · 9 comments · Fixed by #77

Comments

@gentlementlegen
Copy link
Member

The Kernel does not forward commands if the configuration does not list the command, as seen in this code.
It works usually fine except for plugins that can handle multiple commands, such as start stop.

Once the manifests are up and running, it would be important to update the logic so multiple commands can be handled by the same plugin.

Copy link

ubiquibot bot commented Jul 16, 2024

@gentlementlegen, You are not allowed to add Time: <2 Hours

Copy link

ubiquibot bot commented Jul 16, 2024

! No permission to set labels

@0x4007
Copy link
Member

0x4007 commented Jul 17, 2024

such as start stop.

This is half relevant comment, but what if we force each plugin to only support a single command so that they are tightly scoped?

I am conflicted because on one hand I want to make the kernel superior to be able to handle everything. On the other hand, I want to encourage solid plugin development. I don't want to see developers tempted to make one super plugin that has tons of capabilities and is a mess of code etc. More modularity allows partners to more finely customize their configurations for their requirements.

(/stop technically can be replaced by the assignee self unassigning using the GitHub UI.)

@gentlementlegen
Copy link
Member Author

I think in some cases it would lead to a lot of duplicated code, thus lot of duplicated bugs. Maybe start stop is not the best example (plus we talked about assigning without the need to do the /start command just by linking a pull-request), but probably for complex commands it will be a required feature.

@0x4007
Copy link
Member

0x4007 commented Jul 17, 2024

but probably for complex commands it will be a required feature.

Yes but this clearly illustrates that it isn't a priority now right?

Anyways, as long as the changes to the code are not complex, then we can add this capability now. If it is complex, I think its wise to defer until absolutely needed.

@gentlementlegen
Copy link
Member Author

It should be a fairly simple change (< 1h, tops 2h with the related tests), and would be required for our current version of start stop.

Copy link

ubiquibot bot commented Jul 24, 2024

+ Evaluating results. Please wait...

Copy link

ubiquibot-dev bot commented Jul 24, 2024

[ 67.82 WXDAI ]

@gentlementlegen
Contributions Overview
View Contribution Count Reward
Issue Task 1 50
Issue Specification 1 6.5
Issue Comment 2 11.32
Conversation Incentives
Comment Formatting Relevance Reward
The Kernel does not forward commands if the configuration does n…
6.5
content:
  p:
    count: 60
    score: 1
  a:
    count: 5
    score: 1
wordValue: 0.1
formattingMultiplier: 1
1 6.5
I think in some cases it would lead to a lot of duplicated code,…
11.4
content:
  p:
    count: 56
    score: 1
  code:
    count: 1
    score: 1
wordValue: 0.2
formattingMultiplier: 1
0.6 6.84
It should be a fairly simple change (< 1h, tops 2h with the r…
5.6
content:
  p:
    count: 26
    score: 1
  code:
    count: 2
    score: 1
wordValue: 0.2
formattingMultiplier: 1
0.8 4.48

[ 7.74 WXDAI ]

@0x4007
Contributions Overview
View Contribution Count Reward
Issue Comment 2 7.74
Conversation Incentives
Comment Formatting Relevance Reward
This is half relevant comment, but what if we force each plugin …
10.7
content:
  p:
    count: 106
    score: 1
  code:
    count: 1
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.6 6.42
Yes but this clearly illustrates that it isn't a priority now ri…
4.4
content:
  p:
    count: 44
    score: 1
wordValue: 0.1
formattingMultiplier: 1
0.3 1.32

Copy link

ubiquibot bot commented Jul 24, 2024

[ 17.2 WXDAI ]

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment217.2
Conversation Incentives
CommentFormattingRelevanceReward
> such as [start stop](https://github.com/ubiquibot/command-s...
12.7
a:
  count: 1
  score: "1"
  words: 2
code:
  count: 1
  score: "1"
  words: 1
0.712.7
> but probably for complex commands it will be a required fea...
4.50.684.5

[ 83.4 WXDAI ]

@gentlementlegen
Contributions Overview
ViewContributionCountReward
IssueSpecification115
IssueTask150
IssueComment218.4
IssueComment20
Conversation Incentives
CommentFormattingRelevanceReward
The Kernel does not forward commands if the configuration does n...
15
a:
  count: 3
  score: "3"
  words: 5
115
I think in some cases it would lead to a lot of duplicated code,...
12.4
code:
  count: 1
  score: "1"
  words: 1
0.72512.4
It should be a fairly simple change (< 1h, tops 2h with the r...
6
code:
  count: 1
  score: "1"
  words: 2
0.96
I think in some cases it would lead to a lot of duplicated code,...
-
code:
  count: 1
  score: "0"
  words: 1
0.725-
It should be a fairly simple change (< 1h, tops 2h with the r...
-
code:
  count: 1
  score: "0"
  words: 2
0.9-

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants