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

Move jib sync testdata to integration/examples #4367

Merged
merged 4 commits into from
Jun 24, 2020
Merged

Conversation

loosebazooka
Copy link
Member

  • separate out maven/gradle skaffold files into skaffold-maven.yaml
    and skaffold-gradle.yaml
  • add detailed README.md to example
  • does NOT copy example to in /examples yet

@codecov
Copy link

codecov bot commented Jun 22, 2020

Codecov Report

Merging #4367 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4367      +/-   ##
==========================================
- Coverage   71.78%   71.77%   -0.02%     
==========================================
  Files         325      325              
  Lines       12594    12594              
==========================================
- Hits         9041     9039       -2     
- Misses       2978     2979       +1     
- Partials      575      576       +1     
Impacted Files Coverage Δ
...affold/kubernetes/portforward/kubectl_forwarder.go 60.97% <0.00%> (-2.44%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f14d11...8457c1e. Read the comment docs.

integration/examples/jib-sync/README.md Show resolved Hide resolved
}
```

- You must include the `spring-boot-devtools` dependency at the `compile/implementation` scope, which is contrary to the configuration outlined in the [official docs](https://docs.spring.io/spring-boot/docs/current/reference/html/using-spring-boot.html#using-boot-devtools). Because jib is unaware of any special spring only configuration in your builds, we recommend using profiles to turn on or off devtools support in your jib container builds.
Copy link
Member

Choose a reason for hiding this comment

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

I think Jib currently includes spring-boot-devtools even if <optional>true. (And we decided to take care of this using an extension.) Should we still advise to set <scope>compile?

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 think we leave optional dependencies out. no?

Copy link
Member Author

Choose a reason for hiding this comment

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

because it's not explicitly resolved to be a runtime dependency.

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 about the scope, <compile> is default isn't it?

Copy link
Member

@chanseokoh chanseokoh Jun 22, 2020

Choose a reason for hiding this comment

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

Ah, right, compile is already the default. Then, the Spring Boot doc only adds <optional>true, so I think this comment to add it as compile doesn't apply.

And I think we include spring-boot-devtools regardless of <optional>true. (Would be good to double-check.)

Copy link
Member Author

@loosebazooka loosebazooka Jun 23, 2020

Choose a reason for hiding this comment

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

yeah you're right. I can say <!-- optional not necessary -->

integration/examples/jib-sync/README.md Outdated Show resolved Hide resolved
integration/examples/jib-sync/README.md Outdated Show resolved Hide resolved
Copy link
Member

@chanseokoh chanseokoh left a comment

Choose a reason for hiding this comment

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

Mostly looks good to me.

integration/examples/jib-sync/build.gradle Outdated Show resolved Hide resolved
- separate out maven/gradle skaffold files into skaffold-maven.yaml
  and skaffold-gradle.yaml
- add detailed README.md to example
- does NOT copy example to in `/examples` yet
@tejal29 tejal29 merged commit d378d35 into master Jun 24, 2020
@chanseokoh chanseokoh deleted the jib-sync-to-examples branch October 7, 2020 18:14
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