OPRUN-3021: Improve README #529
Conversation
|
@tmshort: This pull request references OPRUN-3021 which is a valid jira issue. In 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 kubernetes/test-infra repository. |
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
bot
added
the
approved
|
Since this is just readme and the sync scripts, we can probably ignore the e2e tests? (i.e. /override) |
|
|
||
| Even if you only have a single commit, this procedure will follow the same | ||
| process that `sync.sh` does, to ensure no steps are missed. | ||
|
|
There was a problem hiding this comment.
Doesn't sync_pop_candidate.sh also make manifests, vendor, etc. in ways that sync.sh does not?
(And also, sync_pop_candidate.sh needs to make verify-manifests in order to satisfy CI.)
There was a problem hiding this comment.
sync.sh calls sync_pop_candidate.sh
But nothing seems to call make verify or make verify-manifests
There was a problem hiding this comment.
I'm going to document the need to do a make verify but fixing the downstreaming scripts to do that is a bit more involved, so I'd rather save that for another PR.
There was a problem hiding this comment.
My point was really more about the statement about 'will follow the same process' when there is a behavior gap between the scripts.
for e.g.:
There was a problem hiding this comment.
But it is the same process. The sync.sh script:
- The upstream repositories are added as remotes to the monorepo.
- The upstream repositories are fetched.
- A merge branch with the current date (sync-YYYY-MM-DD) is created to do the bulk sync.
- Create a list of candidate commits from upstream to downstream via
sync_get_candidates.sh - Apply each commit via
sync_pop_candidate.sh
The sync.sh script literally calls sync_pop_candidate.sh to do all the work for a list of commits (generated by sync_get_candidates.sh).
The problem is that there's additional verification steps via make verify/make verify-manifests, that need to occur.
There was a problem hiding this comment.
Argh! You're completely correct. I missed the call to sync_pop_candidates.sh.
Thanks for the sanity.
There was a problem hiding this comment.
IMHO we should solve the verification "additional steps" problem in changes to sync_pop_candidates.sh in another PR.
|
@tmshort I have no problem with merging it manually as it is just a doc change. |
Add additional information to the README files. Add help for the sync scripts Signed-off-by: Todd Short <todd.short@me.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dinhxuanvu, grokspawn, stevekuznetsov, tmshort 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 |
|
FWIW Tide will re-test even when there are overrides but yea seems fine to override |
|
Also can configure e2e to skip docs |
|
/lgtm re-lgtm-ing |
|
/override e2e-gcp-console-olm |
|
@tmshort: tmshort unauthorized: /override is restricted to Repo administrators, approvers in top level OWNERS file, and the following github teams:openshift: openshift-release-oversight. In 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 kubernetes/test-infra repository. |
|
(looks as though I can't override) |
|
/retest |
|
/override e2e-gcp-olm |
|
@dinhxuanvu: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. In 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 kubernetes/test-infra repository. |
|
/override ci/prow/e2e-gcp-olm |
|
@dinhxuanvu: Overrode contexts on behalf of dinhxuanvu: ci/prow/e2e-gcp-olm In 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 kubernetes/test-infra repository. |
|
@tmshort: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
Add additional information to the README files.
Add help for the sync scripts