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

Odo v3: Move odo registry to odo preference registry, remove unused pref. settings #5428

Conversation

cdrage
Copy link
Member

@cdrage cdrage commented Feb 2, 2022

What type of PR is this:

/kind feature

What does this PR do / why we need it:

This PR does the following:

  • Moves odo registry to odo preference registry
  • Gets rid of unused preference configuration options BuildTimeout and NamePrefix
  • Reorders the parameters for preference for the usage in --help
  • Adds RegistryCacheTime as a parameter that you can set (it was previously hidden before)
  • Adds more description with regarding to parameters

Which issue(s) this PR fixes:

#5402

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

@openshift-ci openshift-ci bot added the kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation label Feb 2, 2022
@netlify
Copy link

netlify bot commented Feb 2, 2022

✔️ Deploy Preview for odo-docusaurus-preview canceled.

🔨 Explore the source changes: fffe0a7

🔍 Inspect the deploy log: https://app.netlify.com/sites/odo-docusaurus-preview/deploys/6203c9ca534acb00071fedcd

@cdrage cdrage changed the title Move registry to preference [WIP] Move registry to preference Feb 2, 2022
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Feb 2, 2022
@odo-robot
Copy link

odo-robot bot commented Feb 2, 2022

OpenShift Windows Tests on commit 0ddb17e finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 2, 2022

Unit Tests on commit 0ddb17e finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 2, 2022

Validate Tests on commit 0ddb17e finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 2, 2022

Kubernetes Tests on commit 0ddb17e finished successfully.
View logs: TXT HTML

@cdrage
Copy link
Member Author

cdrage commented Feb 3, 2022

/retest

@cdrage
Copy link
Member Author

cdrage commented Feb 3, 2022

/retest-required

@cdrage cdrage force-pushed the move-registry-to-preference branch from 6ddf670 to 59cbe66 Compare February 3, 2022 19:40
@cdrage cdrage changed the title [WIP] Move registry to preference Move registry to preference Feb 3, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Feb 3, 2022
@cdrage cdrage changed the title Move registry to preference Odo v3: Move odo registry to odo preference registry, remove unused parameters Feb 3, 2022
@cdrage cdrage changed the title Odo v3: Move odo registry to odo preference registry, remove unused parameters Odo v3: Move odo registry to odo preference registry, remove unused pref. settings Feb 3, 2022
@cdrage cdrage force-pushed the move-registry-to-preference branch 3 times, most recently from cc5bf6b to d3bc310 Compare February 3, 2022 20:08
@cdrage
Copy link
Member Author

cdrage commented Feb 3, 2022

Ready for reviews! No longer WIP and I've updated the description / added more tests.

@feloy @dharmit @kadel @anandrkskd

@cdrage cdrage force-pushed the move-registry-to-preference branch 3 times, most recently from c6eae3e to 1599702 Compare February 7, 2022 18:03
Copy link
Contributor

@feloy feloy left a comment

Choose a reason for hiding this comment

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

Somme remarks about comments, otherwise lgtm

} else {
// Set the required prefix into componentName
prefix = *cfg.NamePrefix()
// V2 Change -> V3
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please remove this commented code?

}
// For now, the --help page is too large listing all unset values too.
// Just show one example of how to unset a value.
//for _, property := range preference.GetSupportedParameters() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure it is necessary to keep old code here. You could remove the code, and explain why we only display for 1st parameter.

<!--
Thank you for opening a PR! Here are some things you need to know before submitting:

1. Please read our developer guideline: https://github.com/redhat-developer/odo/wiki/Developer-Guidelines
2. Label this PR accordingly with the '/kind' line
3. Ensure you have written and ran the appropriate tests: https://github.com/redhat-developer/odo/wiki/Writing-and-running-tests
4. Read how we approve and LGTM each PR: https://github.com/redhat-developer/odo/wiki/PR-Review

Documentation:

If you are pushing a change to documentation, please read: https://github.com/redhat-developer/odo/wiki/Contributing-to-Docs
-->

**What type of PR is this:**

<!--
Add one of the following kinds:
/kind bug
/kind cleanup
/kind tests
/kind documentation

Feel free to use other [labels](https://github.com/redhat-developer/odo/labels) as needed. However one of the above labels must be present or the PR will not be reviewed. This instruction is for reviewers as well.
-->
/kind feature

**What does this PR do / why we need it:**

This PR does the following:
- Moves "registry" to preference
- Gets rid of unused preference configuration options
- Reorders the parameters for preference for the usage in --help

**Which issue(s) this PR fixes:**
<!--
Specifying the issue will automatically close it when this PR is merged
-->

redhat-developer#5402

**PR acceptance criteria:**

- [X] Unit test

- [X] Integration test

- [X] Documentation

**How to test changes / Special notes to the reviewer:**
@cdrage cdrage force-pushed the move-registry-to-preference branch from 1599702 to fffe0a7 Compare February 9, 2022 14:03
@cdrage
Copy link
Member Author

cdrage commented Feb 9, 2022

Thanks for the comments @feloy I've updated the PR as well as made changes based on your feedback.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 9, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@feloy
Copy link
Contributor

feloy commented Feb 9, 2022

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Feb 9, 2022
@openshift-ci
Copy link

openshift-ci bot commented Feb 9, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: feloy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Feb 9, 2022
@feloy
Copy link
Contributor

feloy commented Feb 9, 2022

/test v4.9-integration-e2e

Infra errors

@feloy
Copy link
Contributor

feloy commented Feb 10, 2022

/test v4.9-integration-e2e

        cmd [ps -ef] failed with error command terminated with exit code 1

@feloy
Copy link
Contributor

feloy commented Feb 10, 2022

/test v4.9-integration-e2e

[Fail] odo devfile supported tests odo debug support for devfile components [It] Verify output debug information for java-springboot works 

@openshift-merge-robot openshift-merge-robot merged commit 9fc77ee into redhat-developer:main Feb 10, 2022
kadel pushed a commit to kadel/odo that referenced this pull request Feb 21, 2022
<!--
Thank you for opening a PR! Here are some things you need to know before submitting:

1. Please read our developer guideline: https://github.com/redhat-developer/odo/wiki/Developer-Guidelines
2. Label this PR accordingly with the '/kind' line
3. Ensure you have written and ran the appropriate tests: https://github.com/redhat-developer/odo/wiki/Writing-and-running-tests
4. Read how we approve and LGTM each PR: https://github.com/redhat-developer/odo/wiki/PR-Review

Documentation:

If you are pushing a change to documentation, please read: https://github.com/redhat-developer/odo/wiki/Contributing-to-Docs
-->

**What type of PR is this:**

<!--
Add one of the following kinds:
/kind bug
/kind cleanup
/kind tests
/kind documentation

Feel free to use other [labels](https://github.com/redhat-developer/odo/labels) as needed. However one of the above labels must be present or the PR will not be reviewed. This instruction is for reviewers as well.
-->
/kind feature

**What does this PR do / why we need it:**

This PR does the following:
- Moves "registry" to preference
- Gets rid of unused preference configuration options
- Reorders the parameters for preference for the usage in --help

**Which issue(s) this PR fixes:**
<!--
Specifying the issue will automatically close it when this PR is merged
-->

redhat-developer#5402

**PR acceptance criteria:**

- [X] Unit test

- [X] Integration test

- [X] Documentation

**How to test changes / Special notes to the reviewer:**
cdrage added a commit to cdrage/odo that referenced this pull request Aug 31, 2022
<!--
Thank you for opening a PR! Here are some things you need to know before submitting:

1. Please read our developer guideline: https://github.com/redhat-developer/odo/wiki/Developer-Guidelines
2. Label this PR accordingly with the '/kind' line
3. Ensure you have written and ran the appropriate tests: https://github.com/redhat-developer/odo/wiki/Writing-and-running-tests
4. Read how we approve and LGTM each PR: https://github.com/redhat-developer/odo/wiki/PR-Review

Documentation:

If you are pushing a change to documentation, please read: https://github.com/redhat-developer/odo/wiki/Contributing-to-Docs
-->

**What type of PR is this:**

<!--
Add one of the following kinds:
/kind bug
/kind cleanup
/kind tests
/kind documentation

Feel free to use other [labels](https://github.com/redhat-developer/odo/labels) as needed. However one of the above labels must be present or the PR will not be reviewed. This instruction is for reviewers as well.
-->
/kind feature

**What does this PR do / why we need it:**

This PR does the following:
- Moves "registry" to preference
- Gets rid of unused preference configuration options
- Reorders the parameters for preference for the usage in --help

**Which issue(s) this PR fixes:**
<!--
Specifying the issue will automatically close it when this PR is merged
-->

redhat-developer#5402

**PR acceptance criteria:**

- [X] Unit test

- [X] Integration test

- [X] Documentation

**How to test changes / Special notes to the reviewer:**
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants