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.

Sign up for GitHub

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

Jump to bottom

Enha: Pin/Bundle sentry-cli Version #143

Merged
merged 16 commits into from
Jul 26, 2022
Merged

Enha: Pin/Bundle sentry-cli Version #143

merged 16 commits into from
Jul 26, 2022

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Jul 11, 2022 •
edited

Closes #136

What changed?

  • Pin version in script/sentry-cli.properties
    • We will also use this file to automatically create PRs when a new version is released.
  • Introduce script/sentry-cli-download.sh to download the configures macOS & Win64 binaries.
  • Bundle binary files with the gem by adding executable wrapper that returns the bundled binary path according to the OS.

Whats next?

  • Are there any drawbacks/regressions to this approach or something else we did not think of?
  • Version is pinned to 1.72.0 for now, need to create a separate PR to update to the most recent version.
  • Do we also need to support windows/linux? If so, we also need to bundle those binaries.
  • Since we bundle sentry-cli now, we probably don't need SentryCheckCliInstalledAction anymore. Should we deprecate & remove it?

@denrase denrase marked this pull request as ready for review July 12, 2022 11:42
@denrase
Merge branch 'master' into enha/pin-sentry-cli-version
1c9f301 
# Conflicts:
#	test/sentry-cli-download.sh
#	test/sentry-cli.properties
@denrase
Copy link
Collaborator Author

denrase commented Jul 12, 2022

Relates to getsentry/team-mobile#7

@denrase denrase mentioned this pull request Jul 12, 2022
Merged
2 tasks
philipphofmann
philipphofmann reviewed Jul 13, 2022
View reviewed changes
Copy link
Member

@philipphofmann philipphofmann left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@denrase, please add a note to the ReadMe, that since version 1.13.0 we bundle sentry-cli but you can still use your own local installation. We should also point out how to use your local installation.

Apart from that LGTM, thanks 🙏

@denrase
Copy link
Collaborator Author

denrase commented Jul 18, 2022

@philipphofmann We now also bundle the Winn64 binary and i've extended the integration test GH action to also run on windows-latest.

philipphofmann
philipphofmann approved these changes Jul 19, 2022
View reviewed changes
Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @denrase.

.github/workflows/integration-test.yml Outdated Show resolved Hide resolved
@denrase denrase merged commit c4a343d into master Jul 26, 2022
@denrase denrase deleted the enha/pin-sentry-cli-version branch July 26, 2022 12:46
mikkopiu pushed a commit to mikkopiu/sentry-fastlane-plugin that referenced this pull request Aug 29, 2022
@denrase
Enha: CI Update sentry-cli (getsentry#144)
1a64362 
Blocked: Need to merge Enha: Pin/Bundle sentry-cli Version getsentry#143 first.
Wait until Add Outputs to Update Workflow github-workflows#18 is merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philipphofmann philipphofmann philipphofmann approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pin fastlane plugin version with CLI version
2 participants
@denrase @philipphofmann