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

Added ability for additional logical dependencies. #270

Closed
wants to merge 1 commit into from
Closed

Added ability for additional logical dependencies. #270

wants to merge 1 commit into from

Conversation

ndrone
Copy link

@ndrone ndrone commented Aug 11, 2016

Issue #152

String actualBootVersion = bootVersion ?: metadata.bootVersions.default.id
Version requestedVersion = Version.parse(actualBootVersion)
resolvedDependencies = depIds.collect {
def dependency = metadata.dependencies.get(it)
if (dependency == null) {
throw new InvalidProjectRequestException("Unknown dependency '$it' check project metadata")
}
dependency.resolve(requestedVersion)
dependency = dependency.resolve(requestedVersion)
Copy link
Contributor

@snicoll snicoll Aug 24, 2016

Choose a reason for hiding this comment

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

That's no really what I have in mind for this feature. What you've implemented looks dangerously like "profile" (i.e. you select one box and several boxes are added). What I had in mind was a way to select a box and have several dependencies added to the project (without being displayed upfront). So you select "foo", you only show "foo" but "foo" and "bar" are added to the build. "bar" can't be selected (it's not a "first-class" Dependency, it's hidden behind another one)

Implementing this in the form of a Dependency list with some smart default inheritance should do the trick. Now I understand that's not what you're proposing here and I am not keen to merge it. I guess that's not helping since your fork is using the feature as is at the moment.

What could we do to conciliate those use cases?

Copy link
Author

Choose a reason for hiding this comment

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

I agree, when I first wrote this. I had the dependencies hidden, but the architect's wanted full transparency to the user base. Let me see if I can rework this to fix both our purposes.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Sep 5, 2016
@snicoll
Copy link
Contributor

snicoll commented Nov 4, 2016

It's been a while so I suggest we close this one for now. It needs a rework anyway as we discussed.

@snicoll snicoll closed this Nov 4, 2016
@snicoll
Copy link
Contributor

snicoll commented Nov 4, 2016

It's been a while so I suggest we close this one for now. It needs a rework anyway as we discussed.

@snicoll snicoll added status: declined and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants