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.

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

Add resources DSL #523

Merged
merged 1 commit into from
Aug 10, 2023
Merged

Add resources DSL #523

merged 1 commit into from
Aug 10, 2023

Conversation

ZacSweers
Copy link
Collaborator

This streamlines configuration of enabling androidResources and enforces use of a resource prefix to avoid conflicts.

May your avatars never be wrongly sized again.

This streamlines configuration of enabling `androidResources` and enforces use of a resource prefix to avoid conflicts.

May your avatars never be wrongly sized again.
@github-actions
Copy link

Review:

To improve the code, you can make the following changes:

  1. Add a null check for androidExtension in the resources() function to handle the case when it is null.
  2. Move the null check for androidExtension outside the resources() function to avoid redundant checks.
  3. Use the withType function to apply the changes to the LibraryExtension instead of casting and checking the type manually.

Here's the updated code:

public fun resources(prefix: String) {
  val libraryExtension = androidExtension as? LibraryExtension
    ?: error("slack.android.features.resources() is only applicable in libraries!")

  libraryExtension?.let { extension ->
    extension.withType<LibraryExtension> {
      resourcePrefix = prefix
      buildFeatures { androidResources = true }
    }
  }
}

By using the withType function, we can ensure that the changes are only applied to LibraryExtension and avoid casting and null checks.

@ZacSweers ZacSweers added this pull request to the merge queue Aug 10, 2023
Merged via the queue into main with commit cb63b5b Aug 10, 2023
3 checks passed
@ZacSweers ZacSweers deleted the z/resourcePrefix branch August 10, 2023 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants