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

Prevent binary files from being checked in #1730

Closed
BenHenning opened this issue Aug 27, 2020 · 5 comments · Fixed by #5525
Closed

Prevent binary files from being checked in #1730

BenHenning opened this issue Aug 27, 2020 · 5 comments · Fixed by #5525
Assignees
Labels
enhancement End user-perceivable enhancements. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@BenHenning
Copy link
Member

Checking in binary files will significantly increase the repository size. We need a pre-commit check to prohibit binary files from being added, since we should never add them. In cases when binary files are needed, we should use Git LFS or an alternate solution.

@BenHenning BenHenning added Type: Improvement Priority: Essential This work item must be completed for its milestone. labels Aug 27, 2020
@BenHenning BenHenning added this to the Beta milestone Aug 27, 2020
@BenHenning
Copy link
Member Author

Also: we should have a CI check to verify that across the entire commit log of a PR that no files are binary (otherwise it should start permanently failing since only a force push can remediate such a situation). This is the only time we should ever force push.

@BenHenning
Copy link
Member Author

We may run into an issue with the CI check if it runs across the entirety of develop since there are some binary files in mainline, and we can't force push change those. We may need to build in exceptions for those files.

@BenHenning
Copy link
Member Author

Note also that this issue unblocks opening write access to contributors who work on big projects and can thus benefit from PR chaining (otherwise isaacs/github#959 prevents proper PR chaining when using forks).

@navneetsaluja
Copy link
Contributor

May I work on this? From what I understand, we need to write a pre-commit shell script. I'm confused about which binary files which you are talking about though, could you give me an example?

@rt4914
Copy link
Contributor

rt4914 commented Jan 22, 2021

cc: @BenHenning

@Broppia Broppia added issue_type_infrastructure Impact: Low Low perceived user impact (e.g. edge cases). labels Jul 29, 2022
@BenHenning BenHenning added Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Z-ibt Temporary label for Ben to keep track of issues he's triaged. and removed Impact: Low Low perceived user impact (e.g. edge cases). labels Sep 16, 2022
@BenHenning BenHenning removed this from the Beta milestone Sep 16, 2022
@seanlip seanlip added enhancement End user-perceivable enhancements. and removed issue_type_infrastructure labels Mar 28, 2023
@seanlip seanlip removed the dev_team label Mar 30, 2023
@MohitGupta121 MohitGupta121 added the Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. label Jun 16, 2023
@seanlip seanlip removed the Priority: Essential This work item must be completed for its milestone. label Jun 28, 2023
@adhiamboperes adhiamboperes added this to the 1.0 Global availability milestone Feb 12, 2024
@Rd4dev Rd4dev self-assigned this Sep 4, 2024
Rd4dev added a commit to Rd4dev/oppia-android that referenced this issue Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement End user-perceivable enhancements. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Development

Successfully merging a pull request may close this issue.

8 participants