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

[eas-cli] Refactor getExpoConfig to remove dangerous default #1857

Merged
merged 3 commits into from
May 25, 2023

Conversation

wschurman
Copy link
Member

Why

We had a bug where we were accidentally putting the private config in the public config due to a tough-to-catch refactoring error. This PR is an effort to make refactoring errors like this less likely in the future.

expo/sentry-expo#321 (comment)

Closes ENG-7940.

How

Separate out private and public config getters to make it clearer that there is a difference in name and type from the private and public expo configs. This should help code reviewers catch errors during refactors.

Test Plan

Run all tests.

@wschurman
Copy link
Member Author

/changelog-entry chore Refactor getExpoConfig to remove dangerous default

@github-actions
Copy link

github-actions bot commented May 24, 2023

Size Change: -354 B (0%)

Total Size: 41.5 MB

Filename Size Change
./packages/eas-cli/dist/eas-linux-x64.tar.gz 41.5 MB -354 B (0%)

compressed-size-action

@codecov
Copy link

codecov bot commented May 24, 2023

Codecov Report

Merging #1857 (3c0fd3a) into main (e302390) will decrease coverage by 0.00%.
The diff coverage is 57.15%.

@@            Coverage Diff             @@
##             main    #1857      +/-   ##
==========================================
- Coverage   52.75%   52.74%   -0.00%     
==========================================
  Files         474      474              
  Lines       17050    17064      +14     
  Branches     3405     3410       +5     
==========================================
+ Hits         8993     8999       +6     
- Misses       8040     8048       +8     
  Partials       17       17              
Impacted Files Coverage Δ
packages/eas-cli/src/build/createContext.ts 24.08% <0.00%> (ø)
packages/eas-cli/src/build/runBuildAndSubmit.ts 24.52% <0.00%> (ø)
packages/eas-cli/src/commands/branch/create.ts 45.17% <ø> (ø)
packages/eas-cli/src/commands/branch/delete.ts 30.19% <ø> (ø)
packages/eas-cli/src/commands/branch/list.ts 56.25% <ø> (ø)
packages/eas-cli/src/commands/branch/rename.ts 35.00% <ø> (ø)
packages/eas-cli/src/commands/branch/view.ts 45.84% <ø> (ø)
packages/eas-cli/src/commands/build/cancel.ts 51.32% <ø> (ø)
packages/eas-cli/src/commands/build/configure.ts 40.91% <ø> (ø)
packages/eas-cli/src/commands/build/index.ts 20.54% <ø> (ø)
... and 52 more

@wschurman wschurman marked this pull request as ready for review May 24, 2023 19:57
@dsokal dsokal removed their request for review May 25, 2023 08:26
@github-actions
Copy link

✅ Thank you for adding the changelog entry!

@wschurman wschurman merged commit d42efad into main May 25, 2023
@wschurman wschurman deleted the @wschurman/public-project-config-refactor branch May 25, 2023 15:58
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 this pull request may close these issues.

2 participants