Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Mistobaan/add docwebsite #274

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

Open
Mistobaan wants to merge 1 commit into CarperAI:main
base: main
Choose a base branch
Loading
from Mistobaan:Mistobaan/add-docwebsite

Conversation

@Mistobaan
Copy link
Contributor

@Mistobaan Mistobaan commented Feb 3, 2023 •
edited
Loading

First iteration on the new website documentation
image

https://trlx-docs-preview.readthedocs.io/en/latest/

@Mistobaan Mistobaan force-pushed the Mistobaan/add-docwebsite branch 3 times, most recently from 9af30de to d65cc5f Compare February 4, 2023 06:05
@Mistobaan Mistobaan marked this pull request as ready for review February 4, 2023 20:22
Copy link
Collaborator

Can you state what are the main changes done here? Also it seems that you accidentally reverted a few prior commits off of current master

setup.py Outdated
"flake8 >= 3.8.3",
]
extras["docs"] = ["sphinx==4.0.0", "sphinx_rtd_theme"]
extras["test_prod"] = ["pytest", "pytest-xdist", "pytest-subtests", "parameterized"]
Copy link
Collaborator

@cat-state cat-state Feb 7, 2023

Choose a reason for hiding this comment

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

What does this get us over just using setup.cfg?

Copy link
Contributor Author

@Mistobaan Mistobaan Feb 10, 2023

Choose a reason for hiding this comment

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

setup.cfg is actually the right way. will fix.

Copy link
Collaborator

@jon-tow jon-tow left a comment

Choose a reason for hiding this comment

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

@Mistobaan This looks great 🚀 I left a few comments and change requests when you get a chance. Thank you!

CONTRIBUTING.md Outdated
Comment on lines 23 to 31
1. Setup your environment:

```bash
conda create -n trlx python=3.8 torch torch-cuda=11.7 -c pytorch -c nvidia
git clone https://github.com/CarperAI/trlx
cd trlx
pip install -e ".[dev]"
pre-commit install
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this. Most of it is repeated below.

Copy link
Collaborator

jon-tow commented Mar 2, 2023

Hi, @Mistobaan! Is there anywhere that I can lend a hand to help push this along?

@Mistobaan Mistobaan force-pushed the Mistobaan/add-docwebsite branch from 9146e8b to dd0ecc7 Compare March 5, 2023 20:44
Copy link
Contributor Author

@jon-tow I have to just find some time to reduce the patch and make it more manageable. The first delta is always tricky then the next documentations should be fine
jon-tow reacted with heart emoji

@Mistobaan Mistobaan force-pushed the Mistobaan/add-docwebsite branch from dd0ecc7 to 77d6ba2 Compare March 20, 2023 17:00
Copy link
Contributor Author

Simplified the PR, latest preview here: https://trlx-docs-preview.readthedocs.io/en/latest/

@Mistobaan Mistobaan force-pushed the Mistobaan/add-docwebsite branch from 77d6ba2 to c5cfc13 Compare March 20, 2023 18:06
Copy link
Collaborator

@jon-tow jon-tow left a comment

Choose a reason for hiding this comment

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

Added a few more nits and cleanups to reflect recent changes. The preview docs look great!

@Mistobaan Mistobaan force-pushed the Mistobaan/add-docwebsite branch from 07dc763 to c5cfc13 Compare March 24, 2023 05:40
Copy link
Contributor

Hey what are we doing here

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

Reviewers

@cat-state cat-state cat-state left review comments

@jon-tow jon-tow jon-tow requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /