-
-
Notifications
You must be signed in to change notification settings - Fork 225
Fix: Add distinct set-commits release options #4109
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
Conversation
Add `SentrySetCommitReleaseOptions` to Sentry.targets that can be configured independently of `SentryReleaseOptions`, since it's possible for these to be conflicting. Fixes #4108
| <Message Importance="High" Text="Setting Sentry commits" /> | ||
| <Exec | ||
| Command="$(SentryCLIBaseCommand) releases set-commits $(SentrySetCommitOptions) $(_SentryRelease) $(SentryReleaseOptions)" | ||
| Command="$(SentryCLIBaseCommand) releases set-commits $(SentrySetCommitOptions) $(_SentryRelease) $(SentrySetCommitReleaseOptions)" |
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.
question: Should we prefix this new property with _?
This property is not intended to be set by consuming projects, right?
And we also don't intend to mention this property in the documentation, do we?
Should we then prefix it with _ to communicate "internal use only)?
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.
No it's supposed to be exposed to SDK users, same as the SentryReleaseOptions. It basically let's users specify whatever options they want (anything documented by sentry-cli releases set-commits --help).
For example they could put --ignore-missing in here.
Add
SentrySetCommitReleaseOptionsto Sentry.targets that can be configured independently ofSentryReleaseOptions, since it's possible for these to be conflicting.Fixes #4108
Testing
This was tested manually with the parameters described in the issue that this PR resolves.