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

Update implementation #2

Merged
merged 1 commit into from
Feb 23, 2019
Merged

Update implementation #2

merged 1 commit into from
Feb 23, 2019

Conversation

stefanleh
Copy link
Contributor

  • filtering of configurations
  • catching exceptions of pom resolve logic
  • copy system properties important for pom resolving to context
  • update README.md
  • update Gradle Wrapper to 5.2.1
  • set fixed project name to avoid different named artifact

Sorry for the german comments. Will fix that in 0.3 ...

* filtering of configurations
* catching exceptions of pom resolve logic
* copy system properties important for pom resolving to context
* update README.md
* update Gradle Wrapper to 5.2.1
* set fixed project name to avoid different named artifact
@coveralls
Copy link

Coverage Status

Coverage remained the same at 0.0% when pulling a04faee on stefanleh:master into ef5b2a7 on uklance:master.

@uklance
Copy link
Owner

uklance commented Feb 22, 2019

Thanks for your contribution. I'll hopefully try it out soon and push out a release for you.

Cheers,
Lance

@uklance uklance merged commit a04faee into uklance:master Feb 23, 2019
@uklance
Copy link
Owner

uklance commented Feb 23, 2019

I built and published version 0.2 of this plugin to the plugin portal. Apparently it can take up to 24 hours to be approved.

@stefanleh
Copy link
Contributor Author

Thanks. I appreciate your effort for releasing.

@uklance
Copy link
Owner

uklance commented Feb 24, 2019

I made a couple of changes and release version 0.3

  1. I removed defaultConfigurations.findAll { it.canBeResolved } logic. If you need a custom collection of configurations, please manually set the configurations (see readme)
  2. I added a systemProperties Map to the task (defaults to System.getProperties()) which can be customized.

@stefanleh
Copy link
Contributor Author

stefanleh commented Feb 24, 2019 via email

@stefanleh
Copy link
Contributor Author

stefanleh commented Feb 24, 2019

I just saw that you left the defaultConfugurations handling in place. The defaults dont work with current gradle relaeses, thats the cause i added defaultConfigurations.findAll { it.canBeResolved }. All configuraitons considered have the resolvable. The current default handling will break all the time and is therefore useless. I suggest to remove it completely and rely on proper configuration by the user.

@uklance
Copy link
Owner

uklance commented Feb 24, 2019 via email

@stefanleh
Copy link
Contributor Author

For me it was:

FAILURE: Build failed with an exception.

  • What went wrong:
    Could not determine the dependencies of task ':mavenDependencyExport'.

Resolving configuration 'testRuntimeOnly' directly is not allowed

Guess you dont have testRuntimeOnly in you testcase?

@uklance
Copy link
Owner

uklance commented Feb 24, 2019 via email

@stefanleh
Copy link
Contributor Author

Currently i have to use:

mavenDependencyExport {
    
    for (Configuration conf : project.buildscript.configurations.findAll{ it.canBeResolved }) {
        configuration conf
    }
    for (Configuration conf : project.configurations.findAll{ it.canBeResolved }) {
        configuration conf
    }
    
}

to make it work.
Maybe making the configurations non private would be fine?
Then i can remove at least the foreach loops.

@uklance
Copy link
Owner

uklance commented Feb 24, 2019 via email

@stefanleh
Copy link
Contributor Author

I see following approaches:

  • removing default handling and adding some hints to documentation
  • filtering the defaults with the canBeResolved and add logging about which configs are considered (and also maybe which were dropped)
  • removing default handling and provide an allResolvableConfigurations method which can be used in the plugin configuration in the mavenDependencyExport closure

@uklance
Copy link
Owner

uklance commented Feb 24, 2019 via email

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.

3 participants