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

Improved roborazzi.generateComposePreviewRobolectricTests.packages specification #618

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

TBSten
Copy link

@TBSten TBSten commented Dec 22, 2024

roborazzi.generateComposePreviewRobolectricTests.packages to set android.namespace instead of immediately raising an error if it is not set.

@takahirom
Copy link
Owner

Thanks. I think this change has a minor tradeoff. When users can't take screenshots because of the package name, it can be difficult to troubleshoot the issue. However, if we require setting the package name, users will understand why screenshots are disabled.

What are your thoughts on this problem? It's similar to the CompositionLocal vs. parameter dilemma in Compose.

@TBSten
Copy link
Author

TBSten commented Dec 23, 2024

As you say, it would be better not to set default values for packages on the library side (roborazzi), since there are problems depending on the environment of the project...
It would be better to use a common build configuration within the project.

However, when I try to set default values for roborazzi.packages using composite build in my project, I can't set default values because of a check by the roborazzi gradle plugin.

// roborazzi.packages is checked first and a configuration error occurs.
afterEvaluate {
  if(packages.get().orEmpty().isEmpty()) {
    packages = listOf(androidExtension.namespace)
  }
}

Is it possible to delay the check until the task is performed as my updated Pull Request indicates to address this issue?

@takahirom
Copy link
Owner

Thanks. I think we can set it like packages.set(provider { listOf(android.namespace) }). Could you check it out?

@TBSten
Copy link
Author

TBSten commented Dec 24, 2024

Oh! I was able to set it up successfully! Thank you...!

I wonder when is the best time for Gradle Plugin validation... 🤔 (some things are not wrapped in Property and may get stuck)

@TBSten
Copy link
Author

TBSten commented Dec 24, 2024

I leave it up to you to decide if you want to merge this PR or not.

Thank you for your response!

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