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

KubernetesProcessor - BuildInfo - possible switch of constructor attributes #6679

Closed
rsvoboda opened this issue Jan 21, 2020 · 3 comments · Fixed by #9250
Closed

KubernetesProcessor - BuildInfo - possible switch of constructor attributes #6679

rsvoboda opened this issue Jan 21, 2020 · 3 comments · Fixed by #9250
Assignees
Labels
kind/bug Something isn't working
Milestone

Comments

@rsvoboda
Copy link
Member

Hi @iocanel / @geoand, I'm looking at https://github.com/quarkusio/quarkus/blob/master/extensions/kubernetes/deployment/src/main/java/io/quarkus/kubernetes/deployment/KubernetesProcessor.java#L236 and I think attributes on line 239 and 240 should be switched.

                project.getBuildInfo().getOutputFile(),
                project.getBuildInfo().getClassOutputDir());

BuildInfo constructor has this signature: public BuildInfo(String name, String version, String packaging, String buildTool, Path outputFile, Path classOutputDir, Path resourceDir) {

When looking into BuildInfo sources I'm even more confused by code like this:

public void setResourceOutputDir(Path classOutputDir) {
    this.classOutputDir = classOutputDir;
  }

It's probably working atm because classOutputDir and resourceDir are basically the same for maven.
https://github.com/dekorateio/dekorate/blob/master/core/src/main/java/io/dekorate/project/BuildInfo.java#L54-L55

@rsvoboda rsvoboda added the kind/bug Something isn't working label Jan 21, 2020
@iocanel
Copy link
Contributor

iocanel commented Jan 21, 2020

BuildInfo has two constructors. One that allows specifying different values for classOutputDir and resourceDir and one which assumes that both are the same. Currently, we use the later in quarkus.

So, yes your assumption that it works only when the two are equal is correct. We should change that.

@gsmet
Copy link
Member

gsmet commented Jan 31, 2020

@iocanel do you still have this one on your radar? Maybe you should affect it to you?

@iocanel iocanel self-assigned this Jan 31, 2020
@iocanel
Copy link
Contributor

iocanel commented Jan 31, 2020

@gsmet: Thanks for the reminder. I had completely forgotten about it.

iocanel added a commit to iocanel/quarkus that referenced this issue May 13, 2020
geoand added a commit that referenced this issue May 13, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
fix (#6679): Use of BuildInfo in k8s processor
@gsmet gsmet added this to the 1.5.0 milestone May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants