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.

Sign up for GitHub

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

Jump to bottom

Code snippets shouldn't include the command prompt #12779

Merged
merged 1 commit into from
Mar 7, 2019
Merged

Code snippets shouldn't include the command prompt #12779

merged 1 commit into from
Mar 7, 2019

Conversation

iamneha
Copy link
Contributor

@iamneha iamneha commented Feb 22, 2019 •
edited
Loading

Fixes: #12739
Code snippets shouldn't include the command prompt and Separate commands from output
I'm using this guide for code snippet formatting and separate commands from output

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 22, 2019
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. language/en Issues or PRs related to English language labels Feb 22, 2019
@netlify
Copy link

netlify bot commented Feb 22, 2019 •
edited
Loading

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 3a9d399

https://deploy-preview-12779--kubernetes-io-master-staging.netlify.com

@iamneha iamneha mentioned this pull request Feb 22, 2019
Closed
2 tasks
@Rajakavitha1
Copy link
Contributor

Rajakavitha1 commented Feb 22, 2019 •
edited
Loading

Hi @iamneha , @DanyC97 , @sftim The best approach would be to first create a PR with the details and guidelines in the style guide and then start fixing the pages that are listed out in #12739.

@Rajakavitha1
Copy link
Contributor

@iamneha I am closing this PR for now. After the guidelines are approved in the Style Guide please feel free to open this PR.

@Rajakavitha1
Copy link
Contributor

/close

@k8s-ci-robot
Copy link
Contributor

@Rajakavitha1: Closed this PR.

In response to this:

/close

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.

@zacharysarah
Copy link
Contributor

@Rajakavitha1 Out of curiosity, what are the missing style guidelines that should be addressed first? For context, see #12739 and the current style guide for command prompts.

This PR looks like a work in progress (there are more files to fix before this PR truly "fixes" the list in #12739 (comment)), so I'd like to reopen it.

@zacharysarah zacharysarah reopened this Feb 25, 2019
@Rajakavitha1
Copy link
Contributor

Rajakavitha1 commented Feb 26, 2019 •
edited
Loading

@Rajakavitha1 Out of curiosity, what are the missing style guidelines that should be addressed first? For context, see #12739 and the current style guide for command prompts.

This PR looks like a work in progress (there are more files to fix before this PR truly "fixes" the list in #12739 (comment)), so I'd like to reopen it.

@zacharysarah based on the discussion between @iamneha @DanyC97 and @sftim in #12739 my understanding was they they had to coordinate before picking on the pages they wanted to fix. Also because we were awaiting a conclusion on the discussion in #12783 so that all are clear about the guidelines before making the fixes.

Also, there are multiple PRs for the same issue. For example, #12794, #12793, #12798 and others that also address "Code snippents shouldn't include the command prompt" and this page is also listed in #12739. It is not possible to track all the PRs that are running in parallel, unless it is a coordinated effort.

@zacharysarah
Copy link
Contributor

@Rajakavitha1 That makes sense. Thanks for providing clarity. 🙇

Closing.

@iamneha
Copy link
Contributor Author

iamneha commented Feb 28, 2019

/reopen

@k8s-ci-robot k8s-ci-robot reopened this Feb 28, 2019
@k8s-ci-robot
Copy link
Contributor

@iamneha: Reopened this PR.

In response to this:

/reopen

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.

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 1, 2019
@iamneha iamneha changed the title [WIP] - Code snippents shouldn't include the command prompt Code snippents shouldn't include the command prompt Mar 1, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 1, 2019
@iamneha iamneha changed the title Code snippents shouldn't include the command prompt Code snippets shouldn't include the command prompt Mar 1, 2019
@iamneha
Copy link
Contributor Author

iamneha commented Mar 1, 2019

@Rajakavitha1 @zacharysarah as per the discussion on the issue #12739
I reopened this PR and pushed the rest of the changes.
Please review it

@DanyC97
Copy link
Contributor

DanyC97 commented Mar 1, 2019 •
edited
Loading

@iamneha fyi i already created a PR yesterday which does the same thing #12901 on entire code base

is waiting for final review / approve from a core reviewer

so maybe you want to close this one and keep the effort in the other PR ?

i don't mind which one is which to be honest as long as it gets in and dragging on

@iamneha
Copy link
Contributor Author

iamneha commented Mar 1, 2019

@DanyC97 I really like the effort you put.
As I remember the issue is assigned to me and I started this PR the day it is assigned to me.
I am not sure what confusion happened here and as per the discussion this PR was closed because we were awaiting a conclusion on the discussion in #12783

Let @zacharysarah @Rajakavitha1 @sftim decide what should be done here now.

@DanyC97 DanyC97 mentioned this pull request Mar 1, 2019
Closed
@DanyC97
Copy link
Contributor

DanyC97 commented Mar 1, 2019

no need for an arbiter @iamneha , closed mine in favor of yours, hopefully it will get in soon

@DanyC97
Copy link
Contributor

DanyC97 commented Mar 7, 2019

/sig docs

@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Mar 7, 2019
@zacharysarah
Copy link
Contributor

@iamneha Good work. ✨

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 7, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zacharysarah

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 7, 2019
@k8s-ci-robot k8s-ci-robot merged commit d3cca48 into kubernetes:master Mar 7, 2019
@DanyC97 DanyC97 mentioned this pull request Mar 8, 2019
Merged
@iamneha iamneha mentioned this pull request Mar 8, 2019
Closed
6 tasks
krmayankk pushed a commit to krmayankk/kubernetes.github.io that referenced this pull request Mar 11, 2019
@iamneha
Code snippents shouldn't include the command prompt (kubernetes#12779)
4e1ef6a
yagonobre pushed a commit to yagonobre/website that referenced this pull request Mar 14, 2019
@iamneha @yagonobre
Code snippents shouldn't include the command prompt (kubernetes#12779)
410715b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikedanese mikedanese Awaiting requested review from mikedanese

@piosz piosz Awaiting requested review from piosz

Assignees

@zacharysarah zacharysarah

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@iamneha @Rajakavitha1 @k8s-ci-robot @zacharysarah @DanyC97