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

Multimodule project dependencies in a different layer #1724

Closed
loosebazooka opened this issue May 17, 2019 · 6 comments
Closed

Multimodule project dependencies in a different layer #1724

loosebazooka opened this issue May 17, 2019 · 6 comments
Milestone

Comments

@loosebazooka
Copy link
Member

loosebazooka commented May 17, 2019

For the purpose of this bug, we will call maven module and gradle sub-project dependencies "modules".

Current Layers

  • classes
  • resources
  • dependencies (with modules that are not versioned SNAPSHOT)
  • snapshot-dependencies (with module versioned as SNAPSHOT)

Proposed Layers (1): All module deps should be considered snapshots

  • classes
  • resources
  • dependencies (no module deps)
  • snapshot-dependencies (includes all module deps)

But this can just be achieved by versioning any module as SNAPSHOT, so maybe there's no value in (1)

Proposed Layers (2): Module deps are their own layer

  • classes
  • resources
  • dependencies (no module deps)
  • snapshot-dependencies (no module deps)
  • module dependencies (all module deps)

@GoogleContainerTools/java-tools-build What do you think? (2) sounds good to me

@chanseokoh
Copy link
Member

Both make sense to me. Slight preference for (2). I vaguely remember someone was saying along the line snapshot dependencies and module dependencies may not be at the same level, but I may be wrong.

This is to fix #1436.

@loosebazooka
Copy link
Member Author

Okay so I guess here are my proposals for updating the JavaImageBuilder api, given that we're probably going with style (2) from above.

JavaContainerBuilder.addDependencies(List<Path> dependencies)

will become

  1. JavaContainerBuilder.addDependencies(List<Path> dependencies) // fixed version dependencies
    JavaContainerBuilder.addSnapshotDependencies(List<Path> dependencies) //snapshot or changing dependencies
    JavaContainerBuilder.addProjectDependencies(List<Path> dependencies) //subproject/module dependencies

or

  1. This is a little more complicated, and the more I think about it, the less I like it.
    JavaContainerBuilder.setDependencies(DependencyProvider dependencies)
    

@chanseokoh
Copy link
Member

I like (1). Hopefully this covers most of the use cases.

@loosebazooka
Copy link
Member Author

Yeah this is opinionated, so I think it'll be fine.

@TadCordle
Copy link
Contributor

Yeah, I like 1 as well.

@chanseokoh
Copy link
Member

Fixed by #1780 and #1785, enabled by #1773.

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

No branches or pull requests

3 participants