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

feat: help command manifest #71

Conversation

gentlementlegen
Copy link
Member

Resolves #58

@gentlementlegen
Copy link
Member Author

gentlementlegen commented Jul 10, 2024

What's new

The command description and example is now taken from a manifest.json file from the plugin itself. When it comes to Github action, it is directly read from the repo; for Workers, directly retrieved from their url appending /manifest.json. Commands are sorted alphabetically for convenience.

QA run

Meniole/ubiquibot-kernel#1 (comment)

About the unit tests

bun is broken with msw before release 1.1.9, as described in this blog post due to the node implementation of bun being incomplete. This breaks the tests, because routes are not being mocked. I succeded to make it run locally by manually linking node libs on top of bun, which seems silly to do.
1.1.9 is not released yet, latest bun is still 1.1.18. We can either:

  • install jest just for the testing part, tedious but doable
  • wait for bun to release 1.1.9 (so no idea when this will be, but they seem to iterate quite quick, one release ~2 weeks) and hope it is really fixed

@gentlementlegen gentlementlegen marked this pull request as ready for review July 12, 2024 05:00
src/types/manifest.ts Show resolved Hide resolved
tests/events.test.ts Outdated Show resolved Hide resolved
@whilefoo
Copy link
Contributor

bun is broken with msw before release 1.1.9

I think it's not a good idea to run tests with different runtime than running the bot. Anyway as part of #64 I had to switch to node/jest because crypto is not compatible by Bun

@gentlementlegen
Copy link
Member Author

@whilefoo Are you still using bun for the runtime then or did you remove it? It seems that you are still using it in your PR so we would indeed be using 2 different runtimes. I will add jest here as well.

@whilefoo
Copy link
Contributor

@whilefoo Are you still using bun for the runtime then or did you remove it?

It's only being used as a package manager

@gentlementlegen
Copy link
Member Author

@whilefoo I fixed all the tests within #74

package.json Show resolved Hide resolved
@rndquu
Copy link
Contributor

rndquu commented Jul 16, 2024

@gentlementlegen Could you update readme with an example for manifest.json and 2 use cases where it must be stored (root server folder for worker and root github repository folder for "github action" plugins)?

@gentlementlegen
Copy link
Member Author

@gentlementlegen Could you update readme with an example for manifest.json and 2 use cases where it must be stored (root server folder for worker and root github repository folder for "github action" plugins)?

I will add an example. I also added boilerplate code within the plugin-template. Its location is the same within the repo regardless of the plugin being a Worker or a Github action. The difference is that for a worker the file must be served, see this code.

@rndquu
Copy link
Contributor

rndquu commented Jul 16, 2024

@gentlementlegen Could you update readme with an example for manifest.json and 2 use cases where it must be stored (root server folder for worker and root github repository folder for "github action" plugins)?

I will add an example. I also added boilerplate code within the plugin-template. Its location is the same within the repo regardless of the plugin being a Worker or a Github action. The difference is that for a worker the file must be served, see this code.

Oh, the example exists here so I think updating the kernel's readme with manifest example is not really necessary

Copy link
Contributor

@rndquu rndquu left a comment

Choose a reason for hiding this comment

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

Approving this with failing tests because, as far as I understand, the tests are fixed in #74

@gentlementlegen gentlementlegen merged commit 056989f into ubiquity-os:development Jul 17, 2024
2 of 3 checks passed
@gentlementlegen gentlementlegen deleted the feat/help-command-manifest branch July 17, 2024 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

command array for help menu clarity
4 participants