-
Notifications
You must be signed in to change notification settings - Fork 100
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
[eas-cli] Fix .git
x .easignore
interaction
#2925
Conversation
Size Change: -3.94 kB (-0.01%) Total Size: 53.4 MB
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2925 +/- ##
==========================================
+ Coverage 52.64% 52.66% +0.03%
==========================================
Files 588 588
Lines 23203 23210 +7
Branches 4613 4614 +1
==========================================
+ Hits 12212 12221 +9
+ Misses 10957 10955 -2
Partials 34 34 ☔ View full report in Codecov by Sentry. |
✅ Thank you for adding the changelog entry! |
Subscribed to pull request
Generated by CodeMention |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks ok
@sjchmiela this is a breaking change for us and it is not apparent at first glance how to fix this issue. We have |
Based on http://expo.fyi/eas-build-archive .easignore should never have been used if requireCommit=true, so my perspective was v15 introduced a bug which this fixed. Main purpose of requireCommit is to ensure |
Is it possible in v14 .easignore wasn't being used at all? |
I was on 15.0.12, bumped to 15.0.13 and started getting this error so had to revert. |
Ohh, I see what you're saying — "if a release requires me to change stuff in my code to proceed, it's a breaking change". I get that. What I was going for was "hey guys this Let me write out how I things played out:
What do you think is the best course of action here now? |
Why
Thanks to a conversation on Bluesky it turned out this check is invalid (always true), because
Ignore
hasDEFAULT_IGNORE
, so even if a user had no.git
in.easignore
, especially if they hadrequireCommit: true
,.git
would be removed.How
.easignore
if one hasrequireCommit: true
.Ignore
so we have separatecreateForCopying
(ignores.git
andnode_modules
always) andcreateForChecking
(does not ignore anything by default). I don't like it, but it should work and is the best of what I could come up with without further refactoring.Test Plan
Added more tests.