[eas-cli] Add support for configuring app store connect connection#3558
[eas-cli] Add support for configuring app store connect connection#3558
Conversation
3f62741 to
bbf0b21
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3558 +/- ##
==========================================
+ Coverage 54.86% 55.15% +0.30%
==========================================
Files 836 843 +7
Lines 35901 36167 +266
Branches 7492 7545 +53
==========================================
+ Hits 19694 19946 +252
- Misses 16112 16125 +13
- Partials 95 96 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bbf0b21 to
ee2dc34
Compare
|
Subscribed to pull request
Generated by CodeMention |
douglowder
left a comment
There was a problem hiding this comment.
Looks good overall. Adding @quinlanj for the review.
| ok, | ||
| action, | ||
| project: metadata.fullName, | ||
| connected: link !== null, |
There was a problem hiding this comment.
apart from "connected" maybe we should also mention something like "valid"? for situation when ascAppIdentifier is set, but remoteAppStoreConnectApp is emtpy (like revoked API key)
There was a problem hiding this comment.
ah ok i see ok, hmm, maybe valid would be better? or maybe we shouldn't even have connected because valid: false, connected: true is confusign and maybe we should have status: "not-connected" | "connected" | "invalid"
There was a problem hiding this comment.
Addressed in 00a0b87
Comment: the GQL query returns an error when remoteAppStoreConnectApp cannot be loaded because of the API key being revoked. remoteAppStoreConnectApp is not-null.
| ` ASC App ID: ${chalk.bold(link.ascAppIdentifier)}`, | ||
| ]; | ||
|
|
||
| if (link.remoteAppStoreConnectApp) { |
There was a problem hiding this comment.
I think lack of remoteAppStoreConnectApp should be considered invalid state, wdyt?
| "channel": { | ||
| "description": "manage update channels" | ||
| }, | ||
| "connections": { |
There was a problem hiding this comment.
There was a problem hiding this comment.
I plan on changing that draft to fall under a eas connections: namespace. Something like eas connections:add convex or eas connections:convex init
There was a problem hiding this comment.
👍 @FiberJW let's make our PRs consistent in terms of command naming.
To explain my way of thinking:
- Why
eas connections:asc:connectand noteas connections:connect:asc? I think it's more intuitive when the verb goes last, and everything proceeding narrows down the scope (from most general to most specific). - Why
connectand notinit? Because creating an association is the only thing this command does – no setting up anything on the ASC side etc. - Why
connectand notcreateoradd? One could argue thatconnections:asc:connectis redundant, and that's a valid point. My opinion is thatconnectjust adds extra precision.
I'm open to changes.
There was a problem hiding this comment.
I support this format. "integrations" would also be a good top-level namespace. "eas integrations asc/convex connect" flows well, but we can go with "connections"
32dc9f8 to
00a0b87
Compare
|
✅ Thank you for adding the changelog entry! |
Why
We're adding App Store Connect integration support. The UI for configuring such connection is available in the web dashboard, but it would also be useful to have it in the CLI – this is the goal of this PR.
How
connectionsoclif topic.eas connections:asc:status– check the status of the current project's connection.eas connections:asc:connect– go through the process of connecting the current project with an ASC app.eas connections:asc:disconnect– disconnects the current project from the ASC app.Test Plan
Interactive mode testing:
Non-interactive mode: