-
Notifications
You must be signed in to change notification settings - Fork 31
Standardize bash vs console usage and fix copy multiline #603
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
Conversation
64cc850 to
bb1e465
Compare
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.
Thanks for starting this.
I found myself leaving the same comment so instead of doing that on every instance I'll just write it here.
We only want to use console when we are showing output from the command. Otherwise we should just use bash. Many markdown linters will complain about console blocks without output.
flowchart TD
A[Single line command or multi-line script]
A -->|multi| B[```bash]
A -->|single| C[Does the command have output?]
C -->|no| B
C -->|yes| D[```console]
source/cloud/aws/ec2.md
Outdated
| ```console | ||
| $ ssh -i <path-to-your-ssh-key-dir>/your-key-file.pem ubuntu@<ip address> | ||
| ``` |
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.
Console blocks are for showing a command and it's output. Given that there is no output here it should just stay as bash.
source/cloud/azure/aks.md
Outdated
|
|
||
| ```bash | ||
| az aks create -g <resource group> -n rapids \ | ||
| ```console |
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.
No output, should be bash
source/cloud/azure/aks.md
Outdated
|
|
||
| ```bash | ||
| az aks nodepool add \ | ||
| ```console |
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.
No output should be bash
|
Ok, I modified this PR to make sure it uses console only when there is output, and wrote this on the guidelines in the README I think got them all. |
40a39ff to
dd57307
Compare
jacobtomlinson
left a 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.
Thanks for going over this again, I know it's a bit of a tedious one.
Unfortunately I think you also need to go through and remove the $ from the bash blocks too.
Note: Check conflicts when merging since there are multiple PRs ongoing touching several files.
$in bash/console commands #534This PR
.md.copybutton_line_continuation_character = "\\"In
.mdfiles (it's ok in.ipynbfor some reasons) we can't add comments inline of a multiline bash command like this, but seems a reasonable thing to avoid..shfile.Out of scope: