-
-
Notifications
You must be signed in to change notification settings - Fork 36
Add fallback pattern for org and project in GenerateDistributionPropertiesTask #1000
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
val sentryPropertiesFile = variant?.let { SentryPropertiesFileProvider.getPropertiesFilePath(project, it) } | ||
val sentryProperties = sentryPropertiesFile?.let { PropertiesUtil.loadMaybe(File(it)) } | ||
|
||
// Resolve org slug with fallback chain: ext -> extension -> env -> sentry.properties |
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.
Happy to discuss a different priority here.
Note: we're not getting .sentryclirc
files yet. That can be added later.
…rtiesTask Update GenerateDistributionPropertiesTask to check multiple sources for org and project configuration, following the established pattern used by other tasks. Changes: - Add fallback chain for org: ext -> extension -> env -> sentry.properties - Add fallback chain for project: ext -> extension -> env -> sentry.properties - Auth token remains extension-only (no fallback) - Pass sentryOrg/sentryProject parameters from AndroidComponentsConfig - Add comprehensive tests for all fallback scenarios Resolves EME-397 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
3e29cca
to
d887e8d
Compare
} | ||
task.projectSlug.set(projectProvider) | ||
|
||
// Auth token only from extension (no fallback) |
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.
so this is deliberate? 'cause I think you can also set it via env variable (SENTRY_AUTH_TOKEN) and sentry.properties too. But I guess it's not gonna be the one you need for distribution, right?
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.
yes that’s correct, we’re going to add a separate env variable and extension property for this one in the future which is why it isn’t added here.
// Load sentry.properties if available | ||
val sentryPropertiesFile = | ||
variant?.let { SentryPropertiesFileProvider.getPropertiesFilePath(project, it) } | ||
val sentryProperties = sentryPropertiesFile?.let { PropertiesUtil.loadMaybe(File(it)) } |
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.
wondering if that's fine to load it at configuration phase here, or should we move it to the task action?
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.
Ah good point. This should probably be marked as an input as well.
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.
LGTM, the priority also sounds fine!
…mpatible 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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.
lgtm
Summary
Update
GenerateDistributionPropertiesTask
to check multiple sources fororg
andproject
configuration, following the established pattern used by other tasks likeSentryUploadProguardMappingsTask
.Changes
Fallback Pattern Implementation
For org (organization slug):
ext.sentryOrg
)extension.org
)SENTRY_ORG
)defaults.org
ororg
)For projectName (project slug):
ext.sentryProject
)extension.projectName
)SENTRY_PROJECT
)defaults.project
orproject
)For authToken:
#skip-changelog as this hasn't been released yet
Resolves EME-397
🤖 Generated with Claude Code