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.

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

bash basics tutorial #1197

Merged
merged 4 commits into from
Dec 12, 2023
Merged

bash basics tutorial #1197

merged 4 commits into from
Dec 12, 2023

Conversation

garrettjwilke
Copy link
Contributor

No description provided.

@ahoneybun
Copy link
Member

Looks good live.

Screenshot from 2023-12-05 11-40-36

@ahoneybun ahoneybun force-pushed the bash-script-basics branch 2 times, most recently from 529f8c9 to bfb1350 Compare December 5, 2023 19:36
@ahoneybun ahoneybun requested a review from a team December 5, 2023 20:15
Copy link
Contributor

@thomas-zimmerman thomas-zimmerman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting this live

Copy link
Member

@leviport leviport left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also doesn't appear to be linked to in the TOC. I think it probably should be. No reason to make it harder to find later, IMO. Probably fits nicely under the "About Your Operating System" sub-heading.

content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
@ahoneybun
Copy link
Member

This also doesn't appear to be linked to in the TOC. I think it probably should be. No reason to make it harder to find later, IMO. Probably fits nicely under the "About Your Operating System" sub-heading.

It's mainly here so that it can be linked to from a blog post/social so it is hidden by default.

@leviport
Copy link
Member

leviport commented Dec 6, 2023

Linking from TOC doesn't prohibit also linking from socials.

A scenario, if you will. I see the social post on my phone, think it's cool, and try to find the article later on another device. Would it be an unreasonable assumption on my part that I'll be able to find the article in the TOC? I'm aware that built-in search will pick it up, but Ctrl+F on the page does not. This is actually the way I initially tried finding the article when I was reviewing this PR.

@ahoneybun
Copy link
Member

Linking from TOC doesn't prohibit also linking from socials.

A scenario, if you will. I see the social post on my phone, think it's cool, and try to find the article later on another device. Would it be an unreasonable assumption on my part that I'll be able to find the article in the TOC? I'm aware that built-in search will pick it up, but Ctrl+F on the page does not. This is actually the way I initially tried finding the article when I was reviewing this PR.

Right but I don't think the idea is to have it as a full article. It's mainly that we needed more options to explain bash scripting that our blog does not offer from my understanding.

@leviport
Copy link
Member

leviport commented Dec 6, 2023

Right but I don't think the idea is to have it as a full article. It's mainly that we needed more options to explain bash scripting that our blog does not offer from my understanding.

It's already a full article though, that's why I was so picky in my review. It's good stuff, and I want it to be maximally discoverable. I understand the intended use case for the article, but I don't know what about that use case prohibits the article from being linked in TOC.

@ahoneybun
Copy link
Member

Right but I don't think the idea is to have it as a full article. It's mainly that we needed more options to explain bash scripting that our blog does not offer from my understanding.

It's already a full article though, that's why I was so picky in my review. It's good stuff, and I want it to be maximally discoverable. I understand the intended use case for the article, but I don't know what about that use case prohibits the article from being linked in TOC.

Because it's just here to be linked to from blog posts and such not a full article.

@leviport
Copy link
Member

leviport commented Dec 6, 2023

I've heard that explanation already. I don't think it justifies not linking the article in TOC. What about this article makes it less than a full article? If just need somewhere to host content that renders markdown, why not use a github gist instead of our support docs?

@ahoneybun
Copy link
Member

I've heard that explanation already. I don't think it justifies not linking the article in TOC. What about this article makes it less than a full article? If just need somewhere to host content that renders markdown, why not use a github gist instead of our support docs?

I haven't actually used a gist before but I have seen them. I think we also want it to look good as well with our formatting from the website.

@ahoneybun ahoneybun force-pushed the bash-script-basics branch 2 times, most recently from ba88bbe to a0fd39d Compare December 6, 2023 18:32
Copy link
Member

@leviport leviport left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My prior review appears to not have been fully addressed.

content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
Copy link
Member

@leviport leviport left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closer

content/bash-scripting-basics.md Outdated Show resolved Hide resolved
content/bash-scripting-basics.md Outdated Show resolved Hide resolved
@leviport
Copy link
Member

@ahoneybun let me know if you like the changes I made to that section

@ahoneybun ahoneybun merged commit f14a6b0 into master Dec 12, 2023
3 checks passed
@ahoneybun ahoneybun deleted the bash-script-basics branch December 12, 2023 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants