eas-cli icon indicating copy to clipboard operation
eas-cli copied to clipboard

add eas.json option to skip push notifications setup

Open dsokal opened this issue 1 year ago • 2 comments

Checklist

  • [ ] I've added an entry to CHANGELOG.md if necessary. You can comment this pull request with /changelog-entry [breaking-change|new-feature|bug-fix|chore] [message] and CHANGELOG.md will be updated automatically.

Why

When building a project that doesn't use push notifications, it's really annoying that EAS CLI keeps asking if you want to set up push notifications credentials for your project. This PR introduces an option to permanently skip the question.

How

Give the user an option to skip the push notifications credentials setup forever when we detect they don't have them set up. Persist this choice to eas.json - "cli.setUpPushNotifications": false.

Test Plan

Tested manually.

The prompt looks like this: Screenshot 2022-08-25 at 12 24 58

dsokal avatar Aug 25 '22 10:08 dsokal

Size Change: +911 B (0%)

Total Size: 40.9 MB

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

compressed-size-action

github-actions[bot] avatar Aug 25 '22 10:08 github-actions[bot]

Codecov Report

Merging #1315 (54dc980) into main (98ad979) will decrease coverage by 0.02%. The diff coverage is 36.37%.

:exclamation: Current head 54dc980 differs from pull request most recent head 40d17be. Consider uploading reports for the commit 40d17be to get more accurate results

@@            Coverage Diff             @@
##             main    #1315      +/-   ##
==========================================
- Coverage   51.57%   51.55%   -0.01%     
==========================================
  Files         399      399              
  Lines       14151    14168      +17     
  Branches     2951     2956       +5     
==========================================
+ Hits         7297     7303       +6     
- Misses       6326     6337      +11     
  Partials      528      528              
Impacted Files Coverage Δ
packages/eas-cli/src/build/createContext.ts 26.79% <ø> (ø)
packages/eas-json/src/schema.ts 100.00% <ø> (ø)
packages/eas-json/src/types.ts 100.00% <ø> (ø)
...-cli/src/credentials/ios/IosCredentialsProvider.ts 43.48% <33.34%> (-3.69%) :arrow_down:
packages/eas-cli/src/credentials/context.ts 35.94% <100.00%> (+1.02%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Aug 25 '22 10:08 codecov[bot]

@quinlanj

Another option would be to use a JSON file cache

This was my initial thought when I started implementing this. I decided on eas.json because I thought that when working in a team, this setting could be helpful for other team members as well. If we stored this in the home dir (like in the code you linked), there would be no way to share it with others. I also considered putting this file in the project directory in .expo-shared and naming it sth like eas-cli.json but thought it was a bit of overkill for just one setting. This is how I ended up with cli. setUpPushNotifications .

I agree this is not ideal. If I were to change it, I'd prefer to make the setting committable.

@brentvatne do you have an opinion on this one?

dsokal avatar Aug 26 '22 08:08 dsokal

Remember to also update schema here https://github.com/expo/eas-cli/blob/main/packages/eas-json/schema/eas.schema.json

wkozyra95 avatar Aug 26 '22 08:08 wkozyra95

/changelog-entry new-feature Add eas.json to skip push notifications credentials setup.

dsokal avatar Aug 29 '22 08:08 dsokal