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

Extracting source/resources required for the GradleRunner tests #477

Merged
merged 4 commits into from
Mar 23, 2023

Conversation

cdsap
Copy link
Member

@cdsap cdsap commented Mar 21, 2023

SimpleAndroid class is getting complex, and with more use cases to cover the class is getting hard to maintain.
Before starting the rework of the class(#460) we need to clean up the current class.
This PR extracts classes/resources used in the "Simple" projects run under the Gradle Runner tests

@cdsap cdsap marked this pull request as ready for review March 22, 2023 02:09
@cdsap cdsap requested a review from a team March 22, 2023 02:09
Copy link
Member

@jprinet jprinet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your version is way more readable 👍

'''.stripIndent()
}

static String strings() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this method name a bit confusing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, changed to getXmlStrings

@@ -0,0 +1,13 @@
package org.gradle.android.writer

class Kotlin {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dislike the name Kotlin as a class name. I'm thinking it might be better to group these code writing classes to something like CodeSnippets. Then we can call CodeSnippets.getKotlinDataClass() and CodeSnippets.getJavaClass(). WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good.
Grouped all sources/resources inCodeSnippets c101f75

Copy link
Member

@jprinet jprinet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -1,5 +1,6 @@
package org.gradle.android


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empty blank line

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done ffca293

@cdsap cdsap merged commit 910324a into main Mar 23, 2023
@cdsap cdsap deleted the iv/extracting-test-project-resources-sources branch March 23, 2023 17:34
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.

None yet

3 participants