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

Update README.md #76

Merged
merged 3 commits into from
Apr 22, 2024
Merged

Update README.md #76

merged 3 commits into from
Apr 22, 2024

Conversation

FWuellhorst
Copy link
Contributor

Closes #74

README.md Outdated
@@ -33,8 +33,10 @@ To install all dependencies, you need python installed (>= 3.7).
If you don't have python or don't want to install it, just clone the required libraries manually.
You can extract the relevant information from the `dependencies.cfg` script.

Using the script, it's just:
**Note:** The requirement `Buildings` requires git large-file-storage (`git lfs`). Either install it
or, in a command line on windows, run: `set GIT_LFS_SKIP_SMUDGE=1`.
Copy link
Contributor

Choose a reason for hiding this comment

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

set GIT_LFS_SKIP_SMUDGE is only working if git lfs is already installed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, thanks for checking. Can you add a os.system in the python script to raise an error if git lfs is not installed in that case?

@HvanderStok HvanderStok self-requested a review April 19, 2024 15:41
Copy link
Contributor

@HvanderStok HvanderStok left a comment

Choose a reason for hiding this comment

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

I included the check if git lfs is installed and removed the note in the README.
@FWuellhorst you have a look at it and merge it then.

Copy link
Contributor Author

@FWuellhorst FWuellhorst left a comment

Choose a reason for hiding this comment

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

Nice, thanks for the change! Feel free to merge :)

@HvanderStok HvanderStok merged commit 4b43a9d into main Apr 22, 2024
@HvanderStok HvanderStok deleted the FWuellhorst-patch-1 branch April 22, 2024 08:40
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.

Raise error for missing git-lfs for buildings install
2 participants