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 README for skaffold config #2304

Merged
merged 13 commits into from
Mar 4, 2020
Merged

Conversation

loosebazooka
Copy link
Member

@loosebazooka loosebazooka commented Feb 20, 2020

merge after 2.1.0, README updates for #2292

@loosebazooka loosebazooka added the PR: Merge After Release This PR should only be merged after the release stated in description. label Feb 20, 2020
Copy link
Contributor

@TadCordle TadCordle left a comment

Choose a reason for hiding this comment

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

Should we link down to the skaffold section in the extended usage section?

jib-gradle-plugin/README.md Outdated Show resolved Hide resolved
jib-maven-plugin/README.md Outdated Show resolved Hide resolved
@loosebazooka
Copy link
Member Author

Should we link down to the skaffold section in the extended usage section?

This section?

https:/GoogleContainerTools/jib/tree/readme-changes-skaffold-config/jib-gradle-plugin#extended-usage

@TadCordle
Copy link
Contributor

Yeah, since all the other closures are listed there, maybe we should link to the skaffold one.

@@ -189,6 +190,7 @@ Field | Type | Default | Description
`dockerClient` | [`dockerClient`](#dockerclient-closure) | See [`dockerClient`](#dockerclient-closure) | Configures Docker for building to/from the Docker daemon.
`containerizingMode` | `String` | `exploded` | If set to `packaged`, puts the JAR artifact built by the Gradle Java plugin into the final image. If set to `exploded` (default), containerizes individual `.class` files and resources files.
`allowInsecureRegistries` | `boolean` | `false` | If set to true, Jib ignores HTTPS certificate errors and may fall back to HTTP as a last resort. Leaving this parameter set to `false` is strongly recommended, since HTTP communication is unencrypted and visible to others on the network, and insecure HTTPS is no better than plain HTTP. [If accessing a registry with a self-signed certificate, adding the certificate to your Java runtime's trusted keys](https:/GoogleContainerTools/jib/tree/master/docs/self_sign_cert.md) may be an alternative to enabling this option.
`skaffold` | [`skaffold`](#skaffold-integration) | *None* | Configures the internal skaffold tasks. This configuration should only be used when integrating with [`skaffold`](#skaffold-integration) |
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should move it up with the other closures (underneath dockerClient)

Copy link
Contributor

@TadCordle TadCordle Feb 26, 2020

Choose a reason for hiding this comment

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

Also, similarly to the other closures, we should put

See [`skaffold`](#skaffold-integration)

in the default value field instead of None?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what you mean by field? I think None is appropriate?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh I see, link to the (skaffold-integration) section?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I mean the Default column. Just trying to mimic what the other closures do in the table.

Copy link
Contributor

@TadCordle TadCordle left a comment

Choose a reason for hiding this comment

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

I think we should add periods to the end of each Description in the tables as well.

jib-gradle-plugin/README.md Outdated Show resolved Hide resolved
jib-gradle-plugin/README.md Outdated Show resolved Hide resolved
jib-gradle-plugin/README.md Outdated Show resolved Hide resolved
jib-maven-plugin/README.md Outdated Show resolved Hide resolved
jib-maven-plugin/README.md Outdated Show resolved Hide resolved
jib-maven-plugin/README.md Outdated Show resolved Hide resolved
@chanseokoh chanseokoh removed the PR: Merge After Release This PR should only be merged after the release stated in description. label Mar 3, 2020
@chanseokoh
Copy link
Member

Time to merge this?

@loosebazooka
Copy link
Member Author

Ah yeah, will do after Tad's suggestions go though.

@loosebazooka loosebazooka merged commit 451384f into master Mar 4, 2020
@loosebazooka loosebazooka deleted the readme-changes-skaffold-config branch March 4, 2020 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants