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

[DRAFT] update install instructions #560

Closed
wants to merge 1 commit into from
Closed

Conversation

jdbocarsly
Copy link
Member

@jdbocarsly jdbocarsly commented Feb 2, 2024

Fix some issues with the current install instructions

Copy link

cypress bot commented Feb 2, 2024

Passing run #791 ↗︎

0 26 0 0 Flakiness 0

Details:

Merge bd48e8a into 8583853...
Project: datalab Commit: f67561509e ℹ️
Status: Passed Duration: 01:39 💡
Started: Feb 2, 2024 12:54 AM Ended: Feb 2, 2024 12:56 AM

Review all test suite changes for PR #560 ↗︎

@@ -43,7 +43,7 @@ The server should now be accessible at [http://localhost:5001](http://localhost:

Should you wish to contribute to/modify the Python code, you may wish to perform these extra steps:

1. From within the `pydatalab` folder, run `pipenv install --dev` to pull the development dependencies (e.g., `pre-commit`, `pytest`).
1. From within the `pydatalab` folder, run `pipenv install --dev` to pull the development dependencies (e.g., `pre-commit`, `pytest`). You may also need to install `pre-commit` globally.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is the case, but probably you need to run pipenv run pre-commit install the first time, which we don't say below...

Copy link
Member Author

Choose a reason for hiding this comment

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

I jotted this down as a draft while we were struggling getting it to run on a clean install. I think the problem was that pre-commit didn't work when git commit was run outside of the pipenv. The solution may be to run everything from inside the pipenv.

Copy link
Member

Choose a reason for hiding this comment

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

If you install it the first time inside pipenv I think it should work; pre-commit install is simply copying a bash script into .git/hooks/pre-commit and it "remembers" the Python executable that it was first installed by.

@ml-evs ml-evs closed this May 15, 2024
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.

None yet

2 participants