Appstoreconnect

[dvc94ch]

Moves the appstoreconnect api code into it's own crate and extends it with a cli tool and support for the devices, certificates, bundle ids and profiles apis. To generate a key and certificate you can use the following command:

asconnect generate-key --api-key path.to.unified.api.key --type development output.key.and.certificate.pem

[indygreg]

I glanced at this and am definitely supportive. I recently refactored the app store connect code into its own modules suspecting that we'd grow support for more APIs.

Are you receptive to changing the crate name? asconnect isn't obvious to me. How about app-store-connect?

I have a preference for not using anyhow in the library part of the new crate because it makes granular error handling harder. Are you receptive to preserving the use of thiserror? We don't have to use granular error variants - I just think out of principle we want the ability to exposure granular errors for some failures.

[dvc94ch]

so I refactored the error handling. you're right that the previous error handling could be considered sloppy. we have 4 different error types now:

• InvalidPemPrivateKey
• MissingApiKey
• AppStoreConnectError
• NotarizationError

This allows fine grained error handling without wrapping errors, missing backtraces, etc. And is what I'd consider good error handling. Let me know what you think.

[dvc94ch]

to clarify what I mean. the error type is always anyhow::Result ensuring backtraces are captured properly. the source of an error is always a concrete type that can be matched on using downcast_ref::<T> and additional context is added with context instead of putting it in yet another nested enum. the reason I'm skeptical of a gigantic error enum being a good idea is because unless your library only has a single public function, many of the errors in the enum can never actually occur in most functions anyway. The second reason is the insistence on the rust ecosystem on glorifying error handling which does not provide or preserve adequate backtraces. If someone writes a blog post titled "why backtraces are a thing of the past" providing a good argument for it I might change my mind, but as far as I can tell this anti backtrace consensus is not on solid ground.

[indygreg]

I'm fine with this hybrid error approach. Agree that monolithic errors are not great and that backtraces don't need to be preserved at all costs. Will look at patch today.

[indygreg]

This is on the right track. I don't want to step too much in the way of progress.

Let's fix the missing license headers and the new clippy warnings and merge what we have.

As for the license, feel free to convert the new crate to Apache 2.0 + MIT (like other crates) if you want. Pretty sure it is just the two of us who authored all the migrated code.

[dvc94ch]

Think this one is good to go

[indygreg]

I applied this locally and made a handful of modifications (mostly to add missing license files, a changelog, etc) and was about to push it.

But the CLI is completely broken:

$ cargo run --bin app-store-connect
    Finished dev [unoptimized + debuginfo] target(s) in 0.11s
     Running `target/debug/app-store-connect`
thread 'main' panicked at 'Command app-store-connect: Global arguments cannot be required.

        'api_key' is marked as both global and required', /home/gps/.cargo/registry/src/github.com-1ecc6299db9ec823/clap-4.0.29/src/builder/debug_asserts.rs:258:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I pushed my local branch to the app-store-connect branch. Do you want to fix things before merge or after?

If before, just hard reset your branch to the one I pushed and update this PR. If after, I can push that commit to main whenever.

[dvc94ch]

Shit, was thinking I had to test the global change, but then forgot about it. Thanks for noticing.

[dvc94ch]

@indygreg I think the problem fixed itself with you dependency update. I rebased on main and it all worked fine.

• [0] No argument validation on globals clap-rs/clap#1546

[dvc94ch]

my mistake. the error message changed to not recognizing the provided arg, didn't fix the problem. Fixed now.

[indygreg]

I cherry picked the 2 commits you pushed since the last time I looked at things and then pushed the result.

I'm willing to push a release to crates.io whenever you are. Just let me know by filing a GitHub issue.