-
Notifications
You must be signed in to change notification settings - Fork 249
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
docs/design: Minor documentation fixes #546
docs/design: Minor documentation fixes #546
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.
/lgtm
dc2f731
to
4279010
Compare
Codecov Report
@@ Coverage Diff @@
## master #546 +/- ##
=======================================
Coverage 48.86% 48.86%
=======================================
Files 90 90
Lines 6260 6260
=======================================
Hits 3059 3059
Misses 2519 2519
Partials 682 682 Continue to review full report at Codecov.
|
Signed-off-by: timflannagan <[email protected]>
Signed-off-by: timflannagan <[email protected]>
e96caef
to
580c36f
Compare
Looks like the e2e-* actions are failing due to a recent change: actions/runner-images#2703 |
``` | ||
|
||
Now, a binary named `opm` is now built in current directory and ready to be used. | ||
|
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.
Is there a reason to remove this section?
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.
I was just going based on what Kevin was saying in #546 (comment)
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.
I see. I commented on the PR that Kevin mentioned. I'm against removing those alpha commands from opm
. They are under alpha
and not visible for a reason as they are meant for internal/testing/development use. We don't promote those commands as well because SDK is the official tool.
Those commands are frankly a way for us to rapid-test the changes we make to the library we have in registry repo.
Either way, it is OK to remove that section for now. We can always add it back.
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dinhxuanvu, hasbro17, timflannagan 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 |
/retest Please review the full test history for this PR and help us cut down flakes. |
It looks like this is using the old .github test workflow that was fixed recently due to upgrading the default podman version to the latest version before running the core test content. I tried manually re-running those jobs, but it still seems like it's stuck running an out-of-date workflow. Going to try and close and re-open. |
Interesting, well it looks like the new workflows have been picked up, but we're still silently erroring while installing pre-test depdencies:
|
/override ci/prow/e2e-aws |
@timflannagan: Overrode contexts on behalf of timflannagan: ci/prow/e2e-aws 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 e2e-kind e2e-minikube |
@timflannagan: /override requires a failed status context to operate on.
Only the following contexts were expected:
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. |
/retest Please review the full test history for this PR and help us cut down flakes. |
I'm going to force this through as this only touches documentation and nothing user-facing. I'm not entirely sure if you're able to override any failed tests through Github actions like you are able to do with tests configured by prow. |
Description of the change:
General documentation updates/fixes/etc.
Motivation for the change:
Reviewer Checklist
/docs