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

Refactor directory structure #15

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

akihikokuroda
Copy link

@akihikokuroda akihikokuroda commented Oct 25, 2024

Which issue(s) does this pull-request address?

#14 This is the first proposal.

Closes: #14

Description

Restructure directories to contain most of changes in one tool under one directory

Checklist

  • I have read the contributor guide
  • Linting passes: yarn lint or yarn lint:fix
  • Formatting is applied: yarn format or yarn format:fix
  • Unit tests pass: yarn test:unit
  • E2E tests pass: yarn test:e2e
  • Tests are included
  • Documentation is changed or added
  • Commit messages and PR title follow conventional commits

@grahamwhiteuk
Copy link
Collaborator

@akihikokuroda is this still WIP?

@akihikokuroda
Copy link
Author

It's ready for review and comments. I'll take out WIP, Thanks!

@grahamwhiteuk grahamwhiteuk self-requested a review October 29, 2024 10:21
@grahamwhiteuk
Copy link
Collaborator

Great, thanks. Could you rebase to main? I've just updated the core dependencies across the project.

@grahamwhiteuk grahamwhiteuk changed the title [WIP] Refactor directory structure Refactor directory structure Oct 29, 2024
@grahamwhiteuk
Copy link
Collaborator

Thanks. Could you also go through the PR check list and tick off the items you're able to.

@akihikokuroda
Copy link
Author

E2e tests work except tests that require API key.

@grahamwhiteuk
Copy link
Collaborator

No probs, I'll check those out as part of my review - many thanks for your contribution!

@grahamwhiteuk
Copy link
Collaborator

This PR looks a bit muddled to me as it stands right now. I would recommend the following changes:

Re-instate Global Examples
The examples are intended to show the creation of agents using the tools. Some examples may be specific to an individual tool such as that for Airtable where specific information is required to configure it. Some other examples are generic across multiple tools such as that for ImageDescription and OpenLibrary. This move also makes it unclear what would be run from the default "start" command in package.json.

Re-instate a default agent
Setting the start script to tsx is not what I would expect as a user. The default start script should run an example agent that requires no further configuration from the user.

Don't exclude tests
All tests should be run. Some tests have been excluded in package.json

Locate all tests under the tests subdir
It's confusing to have only e2e tests located under the tests subdir and not the unit tests

Dont use @ imports
There is a specific tsconfig set up such that imports can be namespaced as bee-community-tools similar to the way the library would be used in the wild.

Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
@akihikokuroda
Copy link
Author

@grahamwhiteuk Thanks for review. I believe that all comments have been addressed.

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.

Restructure directories
2 participants