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

Rename parameters to DATA_SOURCE_NAME and DATA_SOURCE_NAMESPACE #398

Merged
merged 1 commit into from
Jan 27, 2022

Conversation

akrejcir
Copy link
Contributor

@akrejcir akrejcir commented Jan 3, 2022

What this PR does / why we need it:
Rename from SRC_PVC_NAME to DATA_SOURCE_NAME,
and from SRC_PVC_NAMESPACE to DATA_SOURCE_NAMESPACE.

Release note:

Renamed parameters in templates.

@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jan 3, 2022
@borod108
Copy link

borod108 commented Jan 3, 2022

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 3, 2022
@ksimon1
Copy link
Member

ksimon1 commented Jan 3, 2022

@akrejcir UI is ready for this change?

@akrejcir
Copy link
Contributor Author

akrejcir commented Jan 4, 2022

They are working on it right now.

@akrejcir
Copy link
Contributor Author

akrejcir commented Jan 4, 2022

Waiting for UI
/hold

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 4, 2022
@dominikholler
Copy link
Collaborator

@akrejcir Can you please share the motivation for this change in the commit message?

@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 7, 2022
@dominikholler
Copy link
Collaborator

@akrejcir Can you please share the motivation for this change in the commit message?
@yaacov Is UI ready to get this update?

@yaacov
Copy link
Contributor

yaacov commented Jan 17, 2022

@yaacov Is UI ready to get this update?

No, unfortunately the current UI assume the PVC source is in a parameter called SRC_PVC_NAME/NAMESPCE

p.s. Once we move to using parameters "as is" this will no longer be a problem, but in 4.10 the parameter name is hardcoded in the UI.

@dominikholler
Copy link
Collaborator

No, unfortunately the current UI assume the PVC source is in a parameter called SRC_PVC_NAME/NAMESPCE
p.s. Once we move to using parameters "as is" this will no longer be a problem, but in 4.10 the parameter name is hardcoded in the UI.

@yaacov Do you plan to modify the name of the parameter in 4.10 ?

@yaacov
Copy link
Contributor

yaacov commented Jan 17, 2022

@yaacov Do you plan to modify the name of the parameter in 4.10 ?

We didn't know about the name change, so we didn't make any plans

Note I:
if we do support it, we will have to support both conventions, and check for both names because we need to be backword compatible,
Note II:
It will be nice if we wait with the name change to 4.11, where we plan to remove the practice of giving meaning to template parameters in the UI (CNV-14778)

cc:// @tnisan

Rename from SRC_PVC_NAME to DATA_SOURCE_NAME,
and from SRC_PVC_NAMESPACE to DATA_SOURCE_NAMESPACE.

Signed-off-by: Andrej Krejcir <akrejcir@redhat.com>
@kubevirt-bot kubevirt-bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 27, 2022
@ksimon1
Copy link
Member

ksimon1 commented Jan 27, 2022

/lgtm
/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ksimon1

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

@kubevirt-bot kubevirt-bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 27, 2022
@ksimon1
Copy link
Member

ksimon1 commented Jan 27, 2022

/hold cancel

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 27, 2022
@kubevirt-bot kubevirt-bot merged commit 3f01179 into kubevirt:master Jan 27, 2022
@akrejcir akrejcir deleted the rename-parameters branch January 27, 2022 17:28
@ksimon1
Copy link
Member

ksimon1 commented Jun 30, 2022

/cherrypick release-v0.19
Backporting forgotten PR

@kubevirt-bot
Copy link
Contributor

@ksimon1: new pull request created: #467

In response to this:

/cherrypick release-v0.19
Backporting forgotten PR

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.

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. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants