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

feat: Introduce 3.x and 4.x style of openshift manifests. #12884

Merged
merged 1 commit into from
Oct 28, 2020

Conversation

iocanel
Copy link
Contributor

@iocanel iocanel commented Oct 22, 2020

This pull request introduces quarkus.openshift.flavor which supports values: v3 and v4 (default).
Based on the specified value:

3.x

  • We add the label app which is needed by the openshift console.
  • We remove optional from secrets and configmaps.

@iocanel
Copy link
Contributor Author

iocanel commented Oct 22, 2020

fyi: @gsmet: we discussed about it on zulip.

import io.quarkus.test.ProdModeTestResults;
import io.quarkus.test.QuarkusProdModeTest;

public class OpenshiftV3Test {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have a corresponding v4 test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

V4 is the default, it has not special configuration

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I know. But do we have a test that asserts the same thing as this test but checks for the v4 conditions?

@gastaldi gastaldi changed the title feat: Introdcue 3.x and 4.x style of openshift manifests. feat: Introduce 3.x and 4.x style of openshift manifests. Oct 22, 2020
gsmet
gsmet previously requested changes Oct 23, 2020
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Spotted some typos.

Agreed with @geoand that we need to be sure things are covered for v4. Maybe they are already but we need to check.

@gsmet
Copy link
Member

gsmet commented Oct 26, 2020

@iocanel what's the status for the tests? Are we good?

@gastaldi gastaldi dismissed gsmet’s stale review October 28, 2020 02:04

Changes addressed

@gastaldi gastaldi merged commit f02d346 into quarkusio:master Oct 28, 2020
@gsmet
Copy link
Member

gsmet commented Oct 28, 2020

@gastaldi I had no answer to my question above.

@iocanel what's the status for the tests? Can you confirm it's taken care of?

@gsmet gsmet added this to the 1.10 - master milestone Oct 28, 2020
@gastaldi
Copy link
Contributor

@gsmet I merged this once I saw tests for v3 and v4, hence why I dismissed your review

@gsmet
Copy link
Member

gsmet commented Oct 28, 2020

Then either you or Ionnis should drop a message please. It's really hard to keep track of things if people just silently do things. Thanks.

@gastaldi
Copy link
Contributor

Will do, but rest assured that I always check if all concerns are resolved before merging a PR (well most of the time :))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants