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

Fix Issue #3325 #5004

Merged
merged 18 commits into from
Nov 26, 2024
Merged

Fix Issue #3325 #5004

merged 18 commits into from
Nov 26, 2024

Conversation

Mr-Imperium
Copy link
Contributor

@Mr-Imperium Mr-Imperium commented Nov 14, 2024

Added a configuration options md file to openhands/docs. Added appropriately to sidebars.ts. Fixes #3325

End-user friendly description of the problem this fixes or functionality that this introduces

  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below
    Added documentation for the configuration options on the docsite.

Resolves #3325

Added a configuration options md file to openhands/docs. Added appropriately to sidebars.ts.
Fixes All-Hands-AI#3325
Copy link
Collaborator

@enyst enyst left a comment

Choose a reason for hiding this comment

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

Thank you for working on this, @Mr-Imperium ! This issue might be tricky, because of just how many configuration options there are.

  • I would mostly suggest to take a look at config.toml.template file, here, we have usually tried to keep it up to date, as an example that users can modify as they need
  • I'm afraid the LLM you used hallucinated some default values 😅. For example, AWS defaults are all None, there's no 'us-west' anywhere. The LLM specific options, including AWS options, are here, the same for others, like prompt caching is by default True.
  • for the record, the config module where all *config.py files are, is the implementation of all these options

@Mr-Imperium Mr-Imperium requested a review from enyst November 15, 2024 18:37
@enyst
Copy link
Collaborator

enyst commented Nov 21, 2024

@Mr-Imperium Sorry, I think there are two files now? Can you please take a look

@Mr-Imperium
Copy link
Contributor Author

@Mr-Imperium Sorry, I think there are two files now? Can you please take a look

Updated, please review them.

@enyst
Copy link
Collaborator

enyst commented Nov 25, 2024

@Mr-Imperium This looks good! Only had very minor tweaks to suggest, please take a quick look if you can.

Happy to approve this PR if that's fine with you. This is a good start to document all these, thank you for doing this!

@Mr-Imperium Mr-Imperium requested a review from enyst November 26, 2024 15:04
@Mr-Imperium
Copy link
Contributor Author

@Mr-Imperium This looks good! Only had very minor tweaks to suggest, please take a quick look if you can.

Happy to approve this PR if that's fine with you. This is a good start to document all these, thank you for doing this!

Happy to have been able to contribute! Just a beginner; your guidance has been invaluable. Thanks!

docs/sidebars.ts Outdated Show resolved Hide resolved
@enyst
Copy link
Collaborator

enyst commented Nov 26, 2024

@mamoodi I think the contents here are great to have and this kind of doc is needed for a long time, so I'd like to take this in.

@enyst enyst merged commit 0aa4a71 into All-Hands-AI:main Nov 26, 2024
15 checks passed
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.

[Documentation]: config.toml options should be documented on the doc web site
3 participants