-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Touchup commands.md #2772
Touchup commands.md #2772
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: TomSweeneyRedHat 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 |
Fixes: #2768 |
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.
Changeds LGTM but the table seems to not be consistently formatted which partially predates the PR. @TomSweeneyRedHat, would you run a formatter on it? VS Code seems to be able to do this.
I take it the Asciinemas are the nonfunctional ones that were archived? |
The video links that I removed were archived and didn't show anything other than "this video has been archived". The remaining links each have a video that still works but most (all?) use kpod insted of podman and need to be redone. However I thought there was some value to keeping them in play still. They'll be my first targets to replace/redo. |
@vrothberg I've run it through the formatter, the unfortunate consequence is every line has now changed and it's difficult to tell what the changes I made are. If you want to keep me honest, the last of my changes before the fomat went to town can be seen at: ed67b0f |
LGTM, thanks a lot @TomSweeneyRedHat. The formatting could have been done in a separate commit to ease reviewing but this looks good to me. |
@TomSweeneyRedHat can you rebase for CI fixes? |
Remove the runlabel command as it's now covered by the containers-runlabel command. Add the play command and remove all of the video links that don't have a video attached to them. Plus a little bit of table definition changes. Signed-off-by: TomSweeneyRedHat <[email protected]>
@mheon ack and repushed. Thanks for the heads up. |
/lgtm |
Remove the runlabel command as it's now covered by the
containers-runlabel command. Add the play command and remove
all of the video links that don't have a video attached to them.
Plus a little bit of table definition changes.
Signed-off-by: TomSweeneyRedHat [email protected]