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

Attach build to docker daemon functionality to gradle task #265

Merged
merged 7 commits into from
May 10, 2018

Conversation

TadCordle
Copy link
Contributor

Part of #243.

There's a lot of duplicate code here, so this introduces a few refactoring TODOs, which I grouped into issue #262.

private static final String CACHE_DIRECTORY_NAME = "jib-cache";

/** {@code User-Agent} header suffix to send to the registry. */
private static final String USER_AGENT_SUFFIX = "jib-gradle-plugin";
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't necessary anymore since it's not using a registry.

import org.gradle.api.tasks.TaskAction;

/** Builds a container image and exports to the default Docker daemon. */
public class BuildDockerTask extends DefaultTask {

/** Directory name for the cache. The directory will be relative to the build output directory. */
private static final String CACHE_DIRECTORY_NAME = "jib-cache";
Copy link
Contributor

Choose a reason for hiding this comment

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

We'd probably want this to be shared and the same with BuildImageTask. Maybe ProjectProperties would be a good place for it, and the tasks would call like ProjectProperties#getCacheDirectory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you want me to do this now or hold off on this until last bullet of #262?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can hold off but maybe add a TODO here or at the top of the class


/** Converts an {@link ImageConfiguration} to an {@link Authorization}. */
@Nullable
private static Authorization getImageAuthorization(ImageConfiguration imageConfiguration) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We could prob just put this in ImageConfiguration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same question about #262

.setJvmFlags(jibExtension.getJvmFlags())
.build();

// Disables annoying Apache HTTP client logging.
Copy link
Contributor

Choose a reason for hiding this comment

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

These HTTP stuff can be removed.

@TadCordle TadCordle merged commit a0ce78d into master May 10, 2018
@TadCordle TadCordle deleted the docker_daemon_tasks branch May 10, 2018 19:39
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.

2 participants