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

chore: update build process #22

Closed
wants to merge 8 commits into from
Closed

chore: update build process #22

wants to merge 8 commits into from

Conversation

aanorbel
Copy link
Member

No description provided.

@aanorbel aanorbel marked this pull request as ready for review July 30, 2024 12:59
@aanorbel aanorbel requested a review from sdsantos July 30, 2024 12:59
@aanorbel aanorbel force-pushed the chore/flavor-update branch from 60b2115 to c5ff1da Compare July 30, 2024 13:16
@@ -21,7 +21,7 @@ jobs:
uses: ./.github/actions/setup

- name: Build Android
run: ./gradlew assemble${{ matrix.type }}
run: ./gradlew clean assemble${{ matrix.type }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's a shame if we clean every time, because that clears the work we cached and slows things down. Can we run a specific gradle task do clean what we need?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes we can run copyCommonResourcesToFlavor

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm also thinking, maybe it can go into the actions/setup?

@@ -5,7 +5,7 @@ android-compileSdk = "34"
android-minSdk = "24"
android-targetSdk = "34"

compose-plugin = "1.7.0-alpha02"
compose-plugin = "1.6.11"
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can go back to the stable version? Nice!

composeApp/build.gradle.kts Outdated Show resolved Hide resolved
@@ -279,3 +273,15 @@ fun copyRecursive(
}
}
}

tasks.register("runDebug", Exec::class) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need this task?

Copy link
Member Author

Choose a reason for hiding this comment

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

This enables us to set up custom gradle tasks with composeApp:runDebug -Porganization=ooni or composeApp:runDebug -Porganization=dw to run the various flavors without needing to edit files.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we add the flag to Android Studio run configuration?

Screenshot 2024-07-30 at 15 18 13

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't tried any option that works

@@ -21,7 +21,7 @@ jobs:
uses: ./.github/actions/setup

- name: Build Android
run: ./gradlew assemble${{ matrix.type }}
run: ./gradlew clean assemble${{ matrix.type }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm also thinking, maybe it can go into the actions/setup?

@@ -279,3 +273,15 @@ fun copyRecursive(
}
}
}

tasks.register("runDebug", Exec::class) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we add the flag to Android Studio run configuration?

Screenshot 2024-07-30 at 15 18 13

@@ -279,3 +272,15 @@ fun copyRecursive(
}
}
}

tasks.register("runDebug", Exec::class) {
dependsOn("clean", "uninstallDebug", "installDebug")
Copy link
Collaborator

@sdsantos sdsantos Jul 30, 2024

Choose a reason for hiding this comment

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

I feel like instead of relying so much on the clean gradle task to copy the resources as we need every time, we should figure out the right gradle task that runs on every build. Isn't the preBuild setup working?

Because running clean often slows down everything.

Copy link
Member Author

Choose a reason for hiding this comment

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

The pre build works locally but for some reason it fails on the ci

Copy link
Collaborator

Choose a reason for hiding this comment

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

If I run gradlew assemble it works fine locally, it grabs the resources.

Copy link
Member Author

Choose a reason for hiding this comment

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

but you can launch the dw flavor

@aanorbel aanorbel force-pushed the chore/flavor-update branch 2 times, most recently from 4410509 to e9339c1 Compare July 30, 2024 19:01
@aanorbel aanorbel force-pushed the chore/flavor-update branch from e9339c1 to 46fabd4 Compare July 30, 2024 19:13

- name: Build iOS
- name: Build iOS Framework
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should have the organization flag to no? Once we fix it...

@aanorbel aanorbel closed this Jul 31, 2024
@aanorbel aanorbel deleted the chore/flavor-update branch August 22, 2024 09: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