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

Remove specific software name from header in docs #795

Merged
merged 3 commits into from
Nov 9, 2023

Conversation

addison-grant
Copy link
Contributor

@addison-grant addison-grant commented Nov 9, 2023

The original reason for this pull request has mostly been resolved in #793, except for the header of the section of interest. The section originally mentioned Python, then the example was updated to concern Node.js. After a little discussion, removing the specific software name from the header was suggested, so that's what I've done here.

Original Pull Request description.

Python is mentioned in the docs, but then the example uses Node JS. I am new to Nix, so I don't want to update the example to fully use Python, so I did the simpler change of fixing the heading and mention of Python to instead say Node JS.

If someone wants to fix the example code, or change the text to NodeJS or Node.JS, or whatever, by all means. My goal here is just to make the docs more clear. Thanks!

I don't know if there's some build process that will fix the Table of Contents on the right-hand side:

Python mentioned in ToC
image

Python is mentioned in the docs, but then the example uses Node JS. I am new to Nix, so I don't want to update the example to fully use Python, so I did the simpler change of fixing the heading and mention of Python to instead say Node JS.

If someone wants to fix the example code, or change the text to NodeJS or Node.JS, or whatever, by all means. My goal here is just to make the docs more clear. Thanks!
@addison-grant addison-grant requested a review from a team as a code owner November 9, 2023 16:42
@fricklerhandwerk
Copy link
Collaborator

fricklerhandwerk commented Nov 9, 2023

The Python bit is an artifact from a rewrite of the article. But it's already fixed here, just from two days ago: #793

Thanks for the attention to detail though!

@addison-grant
Copy link
Contributor Author

Oh, I see. Okay 👍🏼 It does look like #793 missed the header, though.

addison-grant and others added 2 commits November 9, 2023 13:49
There's enough detail about it below, and the specific tech isn't the point of this section anyway.

Co-authored-by: Valentin Gagarin <[email protected]>
@addison-grant addison-grant changed the title Update docs to correctly say Node JS instead of Python Remove specific software name from header in docs Nov 9, 2023
@addison-grant
Copy link
Contributor Author

@fricklerhandwerk, I think this is good to go or ready for review now. Thanks!

@infinisil
Copy link
Member

Generally we prefer not having merge commits in PRs. But since this is just a single change we can also squash merge, so there's no problem with it. Keep this in mind in case you have a multi-commit PR in the future!

@infinisil infinisil merged commit ea8357b into NixOS:master Nov 9, 2023
4 checks passed
@addison-grant
Copy link
Contributor Author

Generally we prefer not having merge commits in PRs. But since this is just a single change we can also squash merge, so there's no problem with it. Keep this in mind in case you have a multi-commit PR in the future!

Okay, I'll keep it in mind!

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.

3 participants