-
-
Notifications
You must be signed in to change notification settings - Fork 51
Add functionality and tests for iOS mobile app upload #320
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
base: master
Are you sure you want to change the base?
Conversation
`Instructions and example for changelog`$
To the changelog entry, please add a link to this PR (consider a more descriptive message):` - Add functionality and tests for iOS mobile app upload(#320)
|
2c731b8
to
b3ec7ef
Compare
b3ec7ef
to
fb98c92
Compare
Helper::SentryConfig.parse_api_params(params) | ||
|
||
# Verify xcarchive path | ||
xcarchive_path = params[:xcarchive_path] |
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.
In most cases the app will be built in a prior "lane" and can be re-used automatically. Check out what we did here: https://github.com/EmergeTools/fastlane-plugin-emerge/blob/main/lib/fastlane/plugin/emerge/actions/emerge_action.rb#L17-L26
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.
good idea. i added this on line 42 as the default value.
def self.available_options | ||
Helper::SentryConfig.common_api_config_items + [ | ||
FastlaneCore::ConfigItem.new(key: :xcarchive_path, | ||
env_name: "SENTRY_XCARCHIVE_PATH", |
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.
Similarly I'm not sure we need an env var for this if we are re-using the existing lane stuff.
ef6db2a
to
59818d9
Compare
59818d9
to
36e430c
Compare
CHANGELOG.md
Outdated
@@ -251,7 +257,7 @@ We bumped the minimum sentry-cli version to `1.70.1` because it includes an esse | |||
|
|||
### Fixes | |||
|
|||
- fix: Upload Dif use '-' instead of '_' ([#99](https://github.com/getsentry/sentry-fastlane-plugin/pull/99)) | |||
- fix: Upload Dif use '-' instead of '\_' ([#99](https://github.com/getsentry/sentry-fastlane-plugin/pull/99)) |
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.
ignore this, cursor just reformatted it :’(
README.md
Outdated
- __include_sources__: Optional. Include sources from the local file system and upload them as source bundles. | ||
- __wait__: Wait for the server to fully process uploaded files. Errors can only be displayed if --wait is specified, but this will significantly slow down the upload process. | ||
- __upload_symbol_maps__: Optional. Upload any BCSymbolMap files found to allow Sentry to resolve hidden symbols, e.g. when it downloads dSYMs directly from App Store Connect or when you upload dSYMs without first resolving the hidden symbols using --symbol-maps. | ||
- **type**: Optional. Only consider debug information files of the given type. By default, all types are considered. Valid options: 'dsym', 'elf', 'breakpad', 'pdb', 'pe', 'sourcebundle', 'bcsymbolmap'. |
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.
ignore this, cursor just reformatted it :’(
i’ll undo this later.
README.md
Outdated
) | ||
``` | ||
|
||
By default the `SharedValue::XCODEBUILD_ARCHIVE` sets the `xarchive_path` parameter if set by a prior lane. |
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.
@trevor-e does this make sense? I wasn’t sure of the proper lingo.
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.
We could mention build_app
as an example of a prior lane.
Helper::SentryConfig.parse_api_params(params) | ||
|
||
# Verify xcarchive path | ||
xcarchive_path = params[:xcarchive_path] |
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.
good idea. i added this on line 42 as the default value.
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.
Would it be possible for us to do some sort of integration test with an actual app project? Do we have a project available to test it locally?
README.md
Outdated
) | ||
``` | ||
|
||
By default the `SharedValue::XCODEBUILD_ARCHIVE` sets the `xarchive_path` parameter if set by a prior lane. |
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.
We could mention build_app
as an example of a prior lane.
Regarding testing, I used the hackernews app built locally: https://github.com/EmergeTools/hackernews You need to use the |
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you label it "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
Hey @runningcode, what's the status on this PR? |
We're waiting on two things,
|
This calls
sentry-cli mobile-app upload
with a path to the xcarchive.The lane looks like this (also documented in README):
The
xcarchive_path
is automatically inferred fromSharedValue::XCODEBUILD_ARCHIVE
and can be overridden.See this internal doc for more details on the implementation of this feature: https://www.notion.so/sentry/1f68b10e4b5d80b1a127fb174f87c555