-
Notifications
You must be signed in to change notification settings - Fork 18
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
Do not rely on replacing GENERATED_RES
to hook up the plugin
#383
Conversation
Codecov Report
@@ Coverage Diff @@
## master #383 +/- ##
============================================
- Coverage 81.08% 79.54% -1.54%
Complexity 109 109
============================================
Files 16 16
Lines 608 621 +13
Branches 93 98 +5
============================================
+ Hits 493 494 +1
- Misses 45 53 +8
- Partials 70 74 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
ab55c55
to
77d6682
Compare
77d6682
to
4f759e4
Compare
117e3a1
to
9aae31b
Compare
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.
Looks good. 🦫 Will test tomorrow. But some first comments:
.wiredWith(EasyLauncherTask::outputDir) | ||
.toCreate(InternalArtifactType.GENERATED_RES) | ||
val apgVersion = androidComponents.pluginVersion | ||
if (apgVersion >= AndroidPluginVersion(7, 4).beta(1)) { // https://issuetracker.google.com/issues/237303854 |
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.
According to the Google issue: it might require beta02?
It should be available in beta02.
if (apgVersion >= AndroidPluginVersion(7, 4).beta(1)) { // https://issuetracker.google.com/issues/237303854 | |
if (apgVersion >= AndroidPluginVersion(7, 4).beta(2)) { // https://issuetracker.google.com/issues/237303854 |
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.
tasks.named("generate${capitalisedVariantName}Resources") { it.dependsOn(task) } | ||
} | ||
} else { | ||
// has side-effects, but "works". @see: https://github.com/usefulness/easylauncher-gradle-plugin/issues/382 |
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.
Maybe print a warning as well?
// has side-effects, but "works". @see: https://github.com/usefulness/easylauncher-gradle-plugin/issues/382 | |
// has side-effects, but "works". @see: https://github.com/usefulness/easylauncher-gradle-plugin/issues/382 | |
println("WARNING: EasyLauncher generation might cause issues with Gradle resValues in AGP 7.3.") | |
println("See: https://github.com/usefulness/easylauncher-gradle-plugin/issues/382") |
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.
I'll consider adding the log later. This block of code is called on each variant, and we'd spam the console with duplicated messages. Most probably it'll have to happen once, when plugin gets applied? or a task is invoked? It also would be nice to have an ability to disable the log message. I'll see if I manage to add this in a day or two and if not then I'll make a release with what's on the branch currently, relying on the fact people will have to read release notes 🤞
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.
Sounds good!
easylauncher/src/main/kotlin/com/project/starter/easylauncher/plugin/EasyLauncherPlugin.kt
Outdated
Show resolved
Hide resolved
Just tested with 7.4. Looks good. Great work. |
…fer using `addGeneratedSourceDirectory` api instead
9aae31b
to
b29ed90
Compare
Prefer using
addGeneratedSourceDirectory
API insteadSee #382 and https://issuetracker.google.com/issues/237421684 for more details