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.

Sign up for GitHub

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

Jump to bottom

Allow users to configure skaffold data exported from jib to skaffold #2292

Merged
merged 15 commits into from
Feb 21, 2020
Merged

Allow users to configure skaffold data exported from jib to skaffold #2292

merged 15 commits into from
Feb 21, 2020

Conversation

loosebazooka
Copy link
Member

@loosebazooka loosebazooka commented Feb 12, 2020 •
edited
Loading

@loosebazooka loosebazooka mentioned this pull request Feb 12, 2020
Closed
@loosebazooka loosebazooka force-pushed the gradle-skaffold-config-options branch 3 times, most recently from f4fa3fe to 8224c60 Compare February 12, 2020 22:28
@loosebazooka loosebazooka added this to the v2.1.0 milestone Feb 13, 2020
@loosebazooka loosebazooka changed the title Allow gradle users to configure skaffold data Allow users to configure skaffold data exported from jib to skaffold Feb 14, 2020
@loosebazooka loosebazooka requested a review from a team February 17, 2020 01:05
@lightoze
Copy link
Contributor

@loosebazooka I tested this patch in my project and it seems to work as intended.

TadCordle
TadCordle reviewed Feb 18, 2020
View reviewed changes
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 bother adding system properties for the new configuration options? Maybe in case this stuff becomes configurable through skaffold?

...en-plugin/src/main/java/com/google/cloud/tools/jib/maven/skaffold/SkaffoldConfiguration.java Show resolved Hide resolved
...on/src/main/java/com/google/cloud/tools/jib/plugins/common/PluginConfigurationProcessor.java Outdated Show resolved Hide resolved
@loosebazooka
Copy link
Member Author

I'm going to avoid properties for now... but nothing stops us from adding them in the future. Although I can see how someone would want different configs for different skaffold runs, but that can always be done with profiles, project-properties.

@loosebazooka
Copy link
Member Author

Uh I added a few new changelog/readme changes, would be helpful if someone looks at them before I submit.

@TadCordle
Copy link
Contributor

Can you move the readme stuff to another PR so we don't merge it until after release?

@loosebazooka
Copy link
Member Author

oh right, good point.

@loosebazooka loosebazooka mentioned this pull request Feb 20, 2020
Merged
TadCordle
TadCordle approved these changes Feb 21, 2020
View reviewed changes
jib-gradle-plugin/CHANGELOG.md Outdated Show resolved Hide resolved
jib-maven-plugin/CHANGELOG.md Outdated Show resolved Hide resolved
loosebazooka and others added 2 commits February 21, 2020 11:23
@loosebazooka @TadCordle
Update jib-maven-plugin/CHANGELOG.md
0dc1b8d 
Co-Authored-By: Tad Cordle <tcordle@google.com>
@loosebazooka @TadCordle
Update jib-gradle-plugin/CHANGELOG.md
98e5098 
Co-Authored-By: Tad Cordle <tcordle@google.com>
@loosebazooka loosebazooka merged commit d6bcf77 into master Feb 21, 2020
@loosebazooka loosebazooka deleted the gradle-skaffold-config-options branch February 21, 2020 19:58
@loosebazooka loosebazooka mentioned this pull request Mar 13, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@briandealwis briandealwis briandealwis approved these changes

@TadCordle TadCordle TadCordle approved these changes

@chanseokoh chanseokoh Awaiting requested review from chanseokoh

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
v2.1.0
Development

Successfully merging this pull request may close these issues.
6 participants
@loosebazooka @lightoze @TadCordle @briandealwis @chanseokoh @googlebot