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

Correct composite builds #143

Merged
merged 2 commits into from
Jul 22, 2024
Merged

Conversation

Goooler
Copy link
Contributor

@Goooler Goooler commented Jul 16, 2024

  • After this, it could be run from the root project.
  • AGP versions used by demo-app and the root project should be kept the same.
  • includeBuild phrases should be placed in pluginManagement block.

@Goooler Goooler requested a review from svenjacobs as a code owner July 16, 2024 00:49
@svenjacobs
Copy link
Owner

@Goooler Thanks for the pull request. Would it be possible now to specify the Reveal dependencies in demo-app via project references and not use the artifacts from Maven Central?

@Goooler
Copy link
Contributor Author

Goooler commented Jul 16, 2024

We have to move demo-app out as a normal subproject instead of a standalone root project, or there will be a circular dependency issue in reverse.

@svenjacobs
Copy link
Owner

In commit afe6b08 I moved the demo app to a separate project. It was a subproject before. There was some reason why I did it but I don't remember. Maybe it was a build issue because of Kotlin Multiplatform, maybe it had something to do with the publication of the library. I don't know.

@Goooler
Copy link
Contributor Author

Goooler commented Jul 17, 2024

Or we can let demo-app include build reveal-root, shared module now depends on core and shapes.

@Goooler Goooler force-pushed the include-build-demo-app branch from 2c34e17 to ec183c5 Compare July 17, 2024 06:10
@Goooler Goooler changed the title Include build demo-app Correct composite builds Jul 17, 2024
@svenjacobs
Copy link
Owner

Or we can let demo-app include build reveal-root, shared module now depends on core and shapes.

I think I like that better. But instead of the substitutes, couldn't we directly use project(":reveal:core") etc?

@Goooler
Copy link
Contributor Author

Goooler commented Jul 17, 2024

I'm not sure how to depend on composite build subprojects instead of module notations.

Goooler added 2 commits July 22, 2024 18:29
- After this, it could be run from the root project.
- AGP versions used by demo-app and the root project should be kept the same.
- `includeBuild` phrases should be placed in `pluginManagement` block.
@svenjacobs svenjacobs force-pushed the include-build-demo-app branch from ec183c5 to 001c925 Compare July 22, 2024 16:29
@svenjacobs svenjacobs added the enhancement New feature or request label Jul 22, 2024
@svenjacobs svenjacobs merged commit 9633f21 into svenjacobs:main Jul 22, 2024
3 checks passed
@Goooler Goooler deleted the include-build-demo-app branch July 22, 2024 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants