use gradle lazy Provider for entrypoint container parameter#4003
use gradle lazy Provider for entrypoint container parameter#4003emmileaf merged 1 commit intoGoogleContainerTools:masterfrom
Conversation
|
@emmileaf would you be able to approve the workflows to run on this PR? |
6f43029 to
6ce197b
Compare
|
@wakingrufus Generally, these workflows will be approved and run as part of reviewing the PR. We have a backlog of issues/feature requests and would appreciate your patience with some delay in reviewing. That said, these presubmit checks run checkstyle, unit, and integration tests for the code changes, which can be verified locally. Please refer to the contribution guide for more detailed instructions, if this is what you are looking to do for this PR. Thanks! |
|
I created #4027 for this pr. I will rebase my PR to fix my commit message to properly link it |
b0c9f3e to
528c0a0
Compare
|
@emmileaf Issue is created, linked, and PR is rebased. |
|
Hi @emmileaf just a friendly reminder of this PR. will you have a chance to review it in the near future? |
emmileaf
left a comment
There was a problem hiding this comment.
The change look good overall, but can you add some test coverage for the lazy evaluation and a note on this enhancement in the README, similar to in #3936? Thanks for the contribution, and apologies for the delay in reviewing!
jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ContainerParameters.java
Show resolved
Hide resolved
528c0a0 to
b19a294
Compare
|
I updated the PR with changes to Docs and a Test. |
There was a problem hiding this comment.
I don’t think the new testLazyEvalForEntryPoint unit test would run successfully - while the presubmit workflows need maintainer permissions to run, please make sure to verify your changes locally with ./gradlew build.
Could you take a look into the testProject used in JibPluginTest and set up its build.gradle file to verify the feature added by this PR? (For example, in testLazyEvalForLabels, the assertions verify that the updated labels specified in the test project’s build.gradle is lazy evaluated as expected (note the project.ext.value change to "updated" in an afterEvaluate block.)
Hope this helps!
b19a294 to
6e0eafb
Compare
Sorry about that. Thanks for the guidance. I have updated the test and verified it is passing locally. |
|
Kudos, SonarCloud Quality Gate passed! |
emmileaf
left a comment
There was a problem hiding this comment.
LGTM - thanks for the contribution @wakingrufus!
use gradle lazy Provider for entrypoint container parameter (fixes #4027)
Thank you for your interest in contributing! For general guidelines, please refer to
the contributing guide.
Before filing a pull request, make sure to do the following:
the style guide.
This helps to reduce the chance of having a pull request rejected.