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

git-lfs dependency not documented #121

Open
chartgerink opened this issue Jan 22, 2024 · 1 comment
Open

git-lfs dependency not documented #121

chartgerink opened this issue Jan 22, 2024 · 1 comment

Comments

@chartgerink
Copy link
Member

To clone the repository, one needs to have git-lfs installed. This was a surprise to me, as the documentation does not mention this at all.

Might it be worth mentioning this dependency? I am honestly also a bit surprised to learn afterwards that all the files are <5MB, which would mean it does not necessarily require git-lfs - but I might be missing some historical context surrounding #87 and before.

@Bisaloo
Copy link
Member

Bisaloo commented Jan 24, 2024

Thanks for the report! Ideally, the dependency on git-lfs should be removed but here is some history behind its presence:

git-lfs is used to store the png images in the vignettes/ folder. While the image size is not absolutely unmanageable, there is a risk they end up increasing the total git repo size a lot since images don't diff well.

Ideally though, we wouldn't store these images at all. The whole idea is to show two reports with different parameters side by side in the intro vignette. Creating screenshots on the fly via webshot doesn't work well due to an annoying interaction on file paths with pkgdown & GHA containers.

We could think about alternatives though. E.g., create the reports and include them in an iframe, or something similar. It's quite likely to bump into the same path issues though 🤔

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

No branches or pull requests

2 participants