-
Notifications
You must be signed in to change notification settings - Fork 263
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
Add support to (not) initialize kn config file #1472
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@boaz0: 0 warnings.
In response to this:
Description
Add support to initialize kn config file or to skip it.
Changes
- Add --init flag
- Add --no-init flag
- By default we read the config file and if it doesn't exist create it
- Use viper to create the config file.
Reference
Fixes #1232
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
Hi @boaz0. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/ok-to-test
Thanks for submission. Left one comment and one question: why would this initialization
not done automatically if the file is not present or cannot be loaded?
CHANGELOG.adoc
Outdated
@@ -16,9 +16,14 @@ | |||
|=== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a new ## Unreleased
table to start collecting changes for the next release?
Thanks @dsimansk I will look at it more (+ rebase my code) and address your comments. |
Codecov Report
@@ Coverage Diff @@
## main #1472 +/- ##
==========================================
- Coverage 79.32% 79.28% -0.04%
==========================================
Files 162 162
Lines 8501 8516 +15
==========================================
+ Hits 6743 6752 +9
- Misses 1076 1081 +5
- Partials 682 683 +1
Continue to review full report at Codecov.
|
Ummm... I think |
The reported coverage is calculated by Codecov standards that doesn't match |
Per our chat with @boaz0 I've suggested the following approach to the config init:
@maximilien @navidshaikh @rhuss @itsmurugappan please state any objections to the above. |
If the config file doesn't exist create one with default settings. I used the settings that are given in this link: https://github.com/knative/client/blob/main/docs/README.md#options Signed-off-by: Boaz Shuster <[email protected]>
The following is the coverage report on the affected files.
|
/retest |
1 similar comment
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
/lgtm
Thanks !
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: boaz0, rhuss 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 |
If the config file doesn't exist create one with default settings. I used the settings that are given in this link: https://github.com/knative/client/blob/main/docs/README.md#options Signed-off-by: Boaz Shuster <[email protected]>
Description
Add support to initialize kn config file or to skip it.Make a default config file with the newest settings if it doesn't exist.
Changes
Add --init flagAdd --no-init flagBy default we read the config file and if it doesn't exist create itreturn null
if config file does not exist, create and populate it with default settingsReference
Related #1232