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

add additional check for files that should be in lfs (even if they are below size threshold) #123

Closed
naseemkullah opened this issue Aug 2, 2021 · 4 comments · Fixed by #124

Comments

@naseemkullah
Copy link
Collaborator

e.g. If a user does not have lfs installed and checks in a jpg which is below the threshold set by this action, this action will pass.

I would like to add an additional check to see if lfs tracked files are indeed being checked in as LFS files, even if they are below the size threshold set by the warning.

I think these two checks are complimentary, where the size one warns users that they should use LFS to store a file (even if its .js or .json, etc.) the check I will be adding will make sure that a file that is lfs tracked is not accidentally committed as a regular binary file.

@ppremk
Copy link
Owner

ppremk commented Aug 2, 2021

@naseemkullah thank you for this issue. I will be allocating some time to look into improvements as suggested. Please bare with me as I complete competing priorities before getting to this task. If you have some bandwidth, I am more than happy to review and approve a pull request ❤️ 🙇🏽

@naseemkullah
Copy link
Collaborator Author

naseemkullah commented Aug 2, 2021

Hello @ppremk I would be happy to! 😄

Would you be alright if I converted to typescript?

edit: I've opened a PR: #124 🙏

@naseemkullah
Copy link
Collaborator Author

As for the implementation, I plan to iterate through the PR files and us git check-attr is used to determine if any of them should be stored in LFS.

Then among the PR files that should be stored in LFS (if any), check for the string version https://git-lfs.github.com/spec/v1 to determine if the file is indeed a pointer or not.

@naseemkullah naseemkullah mentioned this issue Aug 3, 2021
Merged
@naseemkullah
Copy link
Collaborator Author

@ppremk I added the functionality in the same PR that converts to TS, hope that is okay! (they are in separate commits), PTAL 🙏

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 a pull request may close this issue.

2 participants