-
Notifications
You must be signed in to change notification settings - Fork 19
Edit support for proxy configuration in modules for multi module project #146
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
base: main
Are you sure you want to change the base?
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
project.getGradle().projectsEvaluated(projectsEvaluatedBuildListener::projectsEvaluated); | ||
|
||
// Set build started if not set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this change Moving buildStarted info into restricting it for root project required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't this inly suppose to happen only for the build info?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have also checked it is not providing the build info to the root project. On contrary, there is no build info which gets created for the modules. Let me revert back the changes as it is not related to this feature.
orderedTasks.forEach(t -> { | ||
ArtifactoryPluginConvention convention = ExtensionsUtils.getExtensionWithPublisher(t.getProject()); | ||
if (convention != null) { | ||
DeployUtils.deployTaskArtifacts(convention.getClientConfig(), propsRoot, allDeployDetails, t, null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to update config other sub modules as well ? instead of only the configured sub module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as per the current design the submodule config will not get cascaded from the parent module. only if it defined at the root project it will get cascaded in the submodules.
@@ -83,6 +83,7 @@ public static void addExtractModuleInfoTask(TaskProvider<ArtifactoryTask> collec | |||
extractModuleTask.mustRunAfter(project.getTasks().withType(ArtifactoryTask.class)); | |||
}); | |||
TaskProvider<ExtractModuleTask> finalTaskProvider = taskProvider; | |||
// we are not adding the extractModuleTask as a dependency or else it will run before the artifactoryTask, addDeploymentTask will take care of the execution |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while reading code it will be tough to understand what is extractModuleTask, artifactoryTask, addDeploymentTask. Lets elaborate more.
@@ -52,8 +53,9 @@ public FileCollection getModuleInfoFiles() { | |||
public void extractBuildInfoAndDeploy() throws IOException { | |||
log.debug("Extracting build-info and deploying build details in task '{}'", getPath()); | |||
ArtifactoryClientConfiguration accRoot = ExtensionsUtils.getArtifactoryExtension(getProject()).getClientConfig(); | |||
// Deploy Artifacts to artifactory | |||
// We should ensure that the actual project configuration should be passed to deploy task, but it might be possible that publish task for the modules are not evaluated yet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it like "ensure root project configuration should be passed?"
and "it is possible that publish task for the sub modules are not evaluated yet" ? Can you review the comment again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, because at this time it might be possible that artifactory task are not available but orderedTasks.forEach() this will ensure that artifactory task is available for the execution. I can remove this comment as this was for my reference.
tests.
Problem Statement -
The Gradle Artifactory Plugin fails to recognize proxy configurations defined in the build.gradle files for respective modules within a multi-module project.
Solution -
We fix this by ensuring the task receives the module specific properties, allowing it to apply the correct configurations.