CM-641: [release-1.18] Update submodules with latest release-1.18 branch and configure for v1.18.0 release#496
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@chiragkyal: This pull request references CM-641 which is a valid jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@chiragkyal: This pull request references CM-641 which is a valid jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/retest |
chiragkyal
force-pushed
the
update-1.18
branch
from
96576c0 to
e0af672
Compare
|
@chiragkyal: This pull request references CM-641 which is a valid jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
bharath-b-rh
left a comment
bharath-b-rh
left a comment
There was a problem hiding this comment.
lgtm, except for couple of suggestions.
| cd $(cert_manager_submodule_dir); git checkout $(cert_manager_submodule_branch); cd - > /dev/null | ||
| cd $(cert_manager_operator_submodule_dir); git checkout $(cert_manager_operator_submodule_branch); cd - > /dev/null | ||
| cd $(istio_csr_submodule_dir); git checkout $(istio_csr_submodule_tag); cd - > /dev/null |
There was a problem hiding this comment.
The switch-submodules-branch is for updating the submodules with local reference, and update-submodules is for updating with the remote.
| cd $(cert_manager_submodule_dir); git checkout $(cert_manager_submodule_branch); cd - > /dev/null | |
| cd $(cert_manager_operator_submodule_dir); git checkout $(cert_manager_operator_submodule_branch); cd - > /dev/null | |
| cd $(istio_csr_submodule_dir); git checkout $(istio_csr_submodule_tag); cd - > /dev/null | |
| # update with local cache. | |
| git submodule update --recursive |
There was a problem hiding this comment.
I think the checkout commands are essential for switch-submodules-branch as well, because this target needs to work independently. Without the checkout commands, this target would only run git submodule update on whatever branches are currently checked out, and it doesn't fulfill its purpose of switching to the configured branches/tags from .gitmodules.
Both targets ultimately need to switch to the same branches/tags specified in .gitmodules.
git submodule foreach --recursive 'git fetch -t' command is only present in update-submodules, so switch-submodules-branch cannot fetch from remote, and it still use the local git data.
I believe the current implementation should work correctly. Could you suggest when it might work differently?
There was a problem hiding this comment.
switch-submodules-branch is in context with the repository branch and submodule commit it's having, which is captured in README too.
Try switching the repository branch, and check the state of submodules.
There was a problem hiding this comment.
In each release branch the git submodules are configured with equivalent release branch in their respective origin repositories.
Now that we are having a mix of branch and tag, I believe the README needs modification as well. Because running make switch-submodules-branch would be misleading if it only tracks branch, instead of branch+tag as in update-submodules. WDYT?
There was a problem hiding this comment.
IMO, not required, the branch we are referring to in target name is the repository branch not of submodule's
If it helps, I have no objection updating it to something like with equivalent release branch/tag in their respective origin repositories.
There was a problem hiding this comment.
Okay, in that case I will revert the changes done for switch-submodules-branch commands, and will modify only update-submodules
| path = cert-manager-operator | ||
| url = https://github.com/openshift/cert-manager-operator.git | ||
| branch = cert-manager-1.17 | ||
| branch = cert-manager-1.18 |
There was a problem hiding this comment.
| branch = cert-manager-1.18 | |
| tag = v1.18.2 |
There was a problem hiding this comment.
I think you meant to use v1.18.2 tag for https://github.com/openshift/jetstack-cert-manager, right?
There was a problem hiding this comment.
yeah, commented on wrong line.
…odules `git submodule update --remote` command used for updating the submodules with the remote was by default referring `.git/config` instead of `.gitmodules` where specific branch/tag is configured. And `.git/config` doesn't have branch/tag details it updating the modules with the default branch(main) in remote. Signed-off-by: chiragkyal <ckyal@redhat.com>
- update .gitmodules - run make update-submodules Signed-off-by: chiragkyal <ckyal@redhat.com>
Signed-off-by: chiragkyal <ckyal@redhat.com>
Signed-off-by: chiragkyal <ckyal@redhat.com>
Signed-off-by: chiragkyal <ckyal@redhat.com>
Signed-off-by: chiragkyal <ckyal@redhat.com>
chiragkyal
force-pushed
the
update-1.18
branch
from
e0af672 to
896515a
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bharath-b-rh, chiragkyal The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
openshift-merge-bot
Bot
merged commit 8e0ed3d
into
openshift:release-1.18
3 participants
.gitmodules.gitmodulesmake update-submodulestekton/cert-manager-operator*to v1.18.0tekton/cert-manager-istio-csr*to v1.18tekton/jetstack-cert-manager*to v1.18.2Containerfile.cert-manager-operator.bundlefor 1.18/cc @bharath-b-rh