-
Notifications
You must be signed in to change notification settings - Fork 85
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
[ENG-10464][eas-cli] throw error if custom build config is gitignored #2123
[ENG-10464][eas-cli] throw error if custom build config is gitignored #2123
Conversation
changelog-entry chore Throw error if custom build config is gitignored |
Size Change: -1.09 kB (0%) Total Size: 42.3 MB
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2123 +/- ##
==========================================
- Coverage 54.19% 54.18% -0.00%
==========================================
Files 509 509
Lines 18660 18662 +2
Branches 3741 3742 +1
==========================================
Hits 10110 10110
- Misses 8529 8531 +2
Partials 21 21 ☔ View full report in Codecov by Sentry. |
/changelog-entry chore Throw error if custom build config is gitignored |
const maybeMetadata = await validateCustomBuildConfigAsync({ | ||
projectDir, | ||
profile: buildProfile.profile, | ||
vcsClient, | ||
}); |
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.
❤️
@@ -24,6 +31,13 @@ export async function validateCustomBuildConfigAsync( | |||
`Custom build configuration file ${chalk.bold(relativeConfigPath)} does not exist.` | |||
); | |||
} | |||
if (await vcsClient.isFileIgnoredAsync(relativeConfigPath)) { |
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.
May a config file reference other files? Maybe not now, but in the future? Or TypeScript functions? Should we make sure whole .eas
is committed instead (instead of just the config file)?
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.
May a config file reference other files?
https://docs.expo.dev/custom-builds/schema/#import
Or TypeScript functions?
https://docs.expo.dev/custom-builds/schema/#functionsfunction_namepath
Should we make sure whole .eas is committed instead (instead of just the config file)?
Good question 🤔
I believe that we should fail if the currently used config file is git ignored, but maybe display a warning if other files in .eas
are git ignored? Ideally, I feel like we can try to resolve which custom functions will be needed for a given build and which config files will be needed and validate that they are not git ignored, but I think it will require some more changes, and I think that we can merge it as v1 to make it better and not wait with it for other stuff to come along. WDYT?
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.
s/custom build/job for future proofing, but otherwise looks good
9d77517
to
b5e4072
Compare
/changelog-entry chore Throw error if custom build config is gitignored |
✅ Thank you for adding the changelog entry! |
Why
If you create a custom build with git ignored config file build on a server will fail even though local validation passed.
How
Use the VCS client to check if the file is ignored. Throw an error if it is.
Test Plan
Test locally. Git ignore the config. Make sure the build runs fine without git ignored config.