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 git checkout typo in readme #212

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix git checkout typo in readme #212

wants to merge 3 commits into from

Conversation

ezyang
Copy link

@ezyang ezyang commented Oct 18, 2022

No description provided.

@ezyang ezyang requested a review from d4l3k October 18, 2022 13:11
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 18, 2022
@@ -143,11 +143,14 @@ Both `docker` and `pip install` options above are wrappers around the `cmake bui
#### Building

```bash
# Setup deps
conda install -c conda-forge libpython-static
Copy link
Member

@d4l3k d4l3k Oct 18, 2022

Choose a reason for hiding this comment

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

this is mentioned in the section before

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, so like I skipped to the build from source section, it didn't work, and then I had to go back and look for it. Just put it close lol

Copy link
Member

Choose a reason for hiding this comment

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

maybe we just add a line about "make sure to install python static as mentioned earlier"

@@ -143,11 +143,14 @@ Both `docker` and `pip install` options above are wrappers around the `cmake bui
#### Building

```bash
# Setup deps
conda install -c conda-forge libpython-static
Copy link
Member

Choose a reason for hiding this comment

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

maybe we just add a line about "make sure to install python static as mentioned earlier"

@@ -157,6 +160,7 @@ cd multipy/runtime
mkdir build
cd build
# use cmake -DABI_EQUALS_1=ON .. instead if you want ABI=1
# if you built PyTorch from source, pass -DPYTORCH_ROOT=path/to/pytorch
Copy link
Member

Choose a reason for hiding this comment

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

This line isn't needed with recent changes. Also does this work given we always set it in cmake?

facebook-github-bot pushed a commit that referenced this pull request Oct 26, 2022
Summary:
This updates the README with some feedback from #212 as well as adds a CHANGELOG for the 0.1 release.

Pull Request resolved: #229

Test Plan: docs only

Reviewed By: anirbanr-fb-r2p, PaliC

Differential Revision: D40691024

Pulled By: d4l3k

fbshipit-source-id: 7b76f7cbc7dc4dbd243b5879f510e3dd8a7c9b59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants