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

Set up git-lfs and track relevant files #315

Merged
merged 9 commits into from
Feb 14, 2024
Merged

Conversation

pavithraes
Copy link
Member

@pavithraes pavithraes commented Feb 12, 2024

Supersedes #300

Fixes #247

Ref: https://git-lfs.com

@pmeier
Copy link
Member

pmeier commented Feb 12, 2024

Not sure why only the optional dependencies CI job fails, but the error seems relevant:

warning: in the working copy of 'docs/assets/brand/logo-lockup-horizontal/logo-lockup-horizontal.png', CRLF will be replaced by LF the next time Git touches it
warning: in the working copy of 'docs/assets/brand/logo-lockup-vertical/logo-lockup-vertical-white.png', CRLF will be replaced by LF the next time Git touches it
warning: in the working copy of 'docs/assets/brand/logo-lockup-vertical/logo-lockup-vertical.png', CRLF will be replaced by LF the next time Git touches it
warning: in the working copy of 'docs/assets/brand/logo-symbol/logo-symbol-white.png', CRLF will be replaced by LF the next time Git touches it
warning: in the working copy of 'docs/assets/brand/logo-symbol/logo-symbol.png', CRLF will be replaced by LF the next time Git touches it
diff --git a/docs/assets/brand/logo-lockup-horizontal/logo-lockup-horizontal.png b/docs/assets/brand/logo-lockup-horizontal/logo-lockup-horizontal.png
index 51d9ee3..596c2a9 100644
Binary files a/docs/assets/brand/logo-lockup-horizontal/logo-lockup-horizontal.png and b/docs/assets/brand/logo-lockup-horizontal/logo-lockup-horizontal.png differ
diff --git a/docs/assets/brand/logo-lockup-vertical/logo-lockup-vertical-white.png b/docs/assets/brand/logo-lockup-vertical/logo-lockup-vertical-white.png
index 4442c59..531fc46 100644
Binary files a/docs/assets/brand/logo-lockup-vertical/logo-lockup-vertical-white.png and b/docs/assets/brand/logo-lockup-vertical/logo-lockup-vertical-white.png differ
diff --git a/docs/assets/brand/logo-lockup-vertical/logo-lockup-vertical.png b/docs/assets/brand/logo-lockup-vertical/logo-lockup-vertical.png
index 6599a2c..a3cb965 100644
Binary files a/docs/assets/brand/logo-lockup-vertical/logo-lockup-vertical.png and b/docs/assets/brand/logo-lockup-vertical/logo-lockup-vertical.png differ
diff --git a/docs/assets/brand/logo-symbol/logo-symbol-white.png b/docs/assets/brand/logo-symbol/logo-symbol-white.png
index aa5744c..e912dae 100644
Binary files a/docs/assets/brand/logo-symbol/logo-symbol-white.png and b/docs/assets/brand/logo-symbol/logo-symbol-white.png differ
diff --git a/docs/assets/brand/logo-symbol/logo-symbol.png b/docs/assets/brand/logo-symbol/logo-symbol.png
index dfaf456..7ae0974 100644
Binary files a/docs/assets/brand/logo-symbol/logo-symbol.png and b/docs/assets/brand/logo-symbol/logo-symbol.png differ

@pavithraes
Copy link
Member Author

(Note: ⚙️ Push updated logo files is a temporary commit to verify that the "binary files... differ" error suggests that the git-lfs tracked logo files are still lingering somehow.)

@pavithraes
Copy link
Member Author

Well, I don't think I understand git-lfs well enough to know how that error fixed itself. I only deleted the old PR branch and cleaned up my local git caches. 🤷🏼‍♀️

However, I did try re-running the optional-dependencies CI, and it seems to pass consistently now.

@pmeier
Copy link
Member

pmeier commented Feb 14, 2024

Ok, the reason that only the optional dependencies job is failing is because it is the only one which uses a git command:

- name: Test optional dependencies
run: |
python scripts/update_optional_dependencies.py
git diff --exit-code

Still trying to figure out why the error happens though.

@pmeier
Copy link
Member

pmeier commented Feb 14, 2024

Ok, I can reproduce the issue when checking out Ragna on a system that doesn't have git-lfs installed.

*.gif filter=lfs diff=lfs merge=lfs -text
*.png filter=lfs diff=lfs merge=lfs -text
# but keep the brand images as regular images
docs/assets/brand/logo-**/* !filter !diff !merge binary
Copy link
Member

Choose a reason for hiding this comment

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

@pavithraes Found the issue. You reset the type to text here, which led to the files being compared as text although they are binaries.

Copy link
Member

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

Thanks @pavithraes!

@pmeier pmeier merged commit ba755d0 into main Feb 14, 2024
11 checks passed
@pmeier pmeier deleted the pavithraes/git-lfs-part2 branch February 14, 2024 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to handle images as part of the documentation?
2 participants