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

Review needed for knowledge base articles #1977

Closed
fhemberger opened this issue Jan 4, 2019 · 15 comments
Closed

Review needed for knowledge base articles #1977

fhemberger opened this issue Jan 4, 2019 · 15 comments
Labels
content Issues/pr concerning content help wanted

Comments

@fhemberger
Copy link
Contributor

The articles for the knowledge base were donated from Nodejitsu a while back but never received a proper review and thus aren't linked in the website navigation:

https://nodejs.org/en/knowledge/

Those articles need to be reviewed. Are they still relevant? Do they need to be updated for current versions of Node.js? Which articles are relevant for the ongoing website redesign content structure?

/cc @nodejs/website-redesign

@keywordnew
Copy link
Contributor

Thanks for surfacing this content!

@Yash-Handa
Copy link
Contributor

Yash-Handa commented Feb 3, 2019

Hi @chowdhurian and @fhemberger,
I would love to contribute to Node.js org by reviewing these Awesome Articles.
Please let me know if someone else is working on it or not, So that I can Start with it :)

And also some extra questions:

  1. Do all these articles need review or just some of them? https://nodejs.org/en/knowledge/
  2. Do I mention the changes in the articles?
    eg: if a feature, say 'XYZ' has been deprecated then should I do:

The feature 'XYZ' has been deprecated in node version vX.X.X and use the feature 'ABC' instead

OR
Update the Articles as if outdated stuff was never there.

@keywordnew
Copy link
Contributor

@Yash-Handa you can totally get started.

To answer your questions:

  1. These articles were first contributed 3 years ago. Only a few have seen further updates, which I see were more high-level rather than content updates. (They're on github). I think they all need review, but I'd suggest starting by taking on a manageable amount first, say 1 article, and reviewing that.
  2. Yes! And when you're unsure or want extra information, you should totally (at)mention a person for context 😄

I hope this was helpful. Let me know if I've left anything unclear.

@fhemberger
Copy link
Contributor Author

Thanks @Yash-Handa! I think if content is deprecated (technically) or no longer useful/best practice, it should be removed.

It would be good if we get a first impression how much of the content is still relevant/up to date. If you're unsure, just ask here and we can find people working on that particular topic who know the details.

@Yash-Handa
Copy link
Contributor

Yash-Handa commented Feb 4, 2019

TL;DR : Will submit a structured Summary (with suggestions) for all the Articles of the knowlege directory in 2 to 3 days

Thanks, @fhemberger, and @chowdhurian for such prompt response :)

I was thinking of something similar to this. Like giving a detailed report about the relevance of the Articles and what changes should be made to them.

As many Articles are deprecated not because of the Node features been removed but because of the libraries/packages they talk about are either outdated/deprecated/not-maintained or some new features and best-practices have been added in the newer versions of the package.

eg: nodejs.org/locale/en/knowledge/command-line/how-to-parse-command-line-arguments.md talks about package optimist which is deprecated now, So suggestions for which packge to use in the Article might be helpful (eg: yargs).

So, will submit a structured summary for the entire directory knowlege in 2 to 3 days

I hope this might fasten up the reviewing process :)

@Yash-Handa
Copy link
Contributor

Yash-Handa commented Feb 5, 2019

Suggestion: For better efficiency, I suggest working on smaller chunks of articles (say 10 to 12 articles at a time), this will allow better granularity, monitoring and focused discussions.

Below is the summary of 12 Articles [from command-line, child-processes, cryptography, and javascript-conventions directories]

command-line (updated 3yrs ago)

  • how-to-get-colors-on-the-command-line

    • RELEVANT (minor changes needed)
    • This article is all about adding colors to the command-line using colors.js library which is among top 3 packages on npm for this job.
    • Suggestion: Add latest features of color.js to the Article like applying background colors, newer syntax, and structured feature list.
  • how-to-parse-command-line-arguments

    • DEPRECATED (OPTIMIST is deprecated)
    • The initial part of the Article which deals with process.argv is still relevant and quite beginner friendly but the latter part is where flags are used is outdated because of the optimist package being used in the example.
    • Suggestion: Should update the later part of the Article for using some other npm package for this purpose like yargs.js
  • how-to-prompt-for-command-line-input

    • DEPRECATED (readline module should be used)
    • The Article uses process.stdin directly for taking the input rather than readline module which is built for this purpose only.
    • It also uses prompt package which has not been updated for the past 2 years
    • Suggestion: should use readline module and Inquirer.js package instead of prompt. There are some deadlinks which require relevant resources to point to.

child-processes (updated 3yrs ago)

  • how-to-spawn-a-child-process

    • RELEVANT (could be more informative)
    • The Article is fully functional and does exactly what it says.
    • Suggestion: Could have one more example, and some details about other spawning functions like. spawn(), .fork(), .execFile() and their sync counter-parts could be added.
    • Note: The Article is complete, the suggestions above are mere enhancements

cryptography (updated 7 months ago)

  • how-to-use-crypto-module

    • DEPRECATED (ciphers section is outdated)
    • The initial part of the Article talks about cryptos and basic definitions and tools which are quite relevant
    • All links to the nodejs.org go to older API.
    • crypto.createCipher(algorithm, key) is deprecated.
    • optimist package is also deprecated.
    • Suggestions: Link to the newer docs, crypto.createCipheriv() should be used and other replacement for optimist package (like yargs.js)
  • how-to-use-the-tls-module

    • RELEVANT(starttls section needs re-writing)
    • The basic concepts are relevant but references to node’s tls module need re-writing
    • Note: I don’t have enough knowledge/experience with tls module to incorporate all best practices of it. Any assistance from a security/crypto expert would be highly appreciated.

javascript-conventions (updated 6 months ago)

  • how-to-create-default-parameters-for-functions

    • IRRELEVANT(it’s in ES6 specification)
    • The article is all about workarounds(hacks) to use default parameter like feature in ES5 and below.
    • Suggestions: This stack overflow answer is all that we need.
  • using-ECMA5-in-nodejs

    • IRRELEVANT(Similar doc exists in nodejs.org website)
    • This Article covers ECMA5 but a general article already exists in nodejs.org website here
    • node.green is another great source to use.
  • what-are-the-built-in-timer-functions

    • RELEVANT (minor addition needed)
    • The Article is fully functional and does exactly what it says.
    • Suggestions: some reference to setImmediate() and process.nextTick() could be made for the sake of completion.
  • what-are-truthy-and-falsy-values

    • RELEVANT
    • This Article deals with the fundamental topic of truthy and falsy values in javascript which hasn’t changed in previous years.
  • what-is-json

    • RELEVANT
    • This Article deals with the fundamentals of JSON which haven’t changed in previous years.
    • Suggestions: link to json.org can be added, link to a JSON validator can also be added like jsonlint
  • what-is-the-arguments-object

    • RELEVANT (minor addition needed)
    • The Article is fully functional and does exactly what it says.
    • Suggestions: A note to indicate that argument object is not available in the new arrow functions introduced with ECMA6.

Hey, @fhemberger, @chowdhurian or anyone with some suggestion/enhancement/tips please feel free to comment below, would love to incorporate those changes in the articles 😄

I would follow one PR per Article as mentioned in Knowledge base articles need to be reviewed ahead of pulling them in
Will submit updated command-line > how-to-get-colors-on-the-command-line tomorrow :)

@fhemberger
Copy link
Contributor Author

@Yash-Handa Wow, amazing! Thank you for all your work!

Yash-Handa added a commit to Yash-Handa/nodejs.org that referenced this issue Feb 7, 2019
Hey @chowdhurian and @fhemberger,
In accordance with issue nodejs#1977, I have updated the Article **how-to-parse-command-line-arguments** with the following changes:

- Exchanged **optimist** with **yargs.js** 
- Added more featured example with yargs.js.
- some other miscellaneous updates and refactors.

If there are any **suggestion/enhancement/tips** please feel free to tell me, I would love to incorporate those changes in this Article 😄
fhemberger pushed a commit that referenced this issue Feb 11, 2019
In accordance with issue #1977, I have updated the Article **how-to-parse-command-line-arguments** with the following changes:

- Exchanged **optimist** with **yargs.js** 
- Added more featured example with yargs.js.
- some other miscellaneous updates and refactors.
ghost pushed a commit that referenced this issue Feb 12, 2019
In accordance with issue #1977, I have updated the Article **how-to-prompt-for-command-line-input** with the following changes:

1. Exchanged **direct stdin usage** with **readline module**
2. Added example of `readline` module.
3. Some other miscellaneous updates and refactors.
ghost pushed a commit that referenced this issue Feb 14, 2019
In accordance with issue #1977, I have updated the Article how-to-prompt-for-command-line-input with the following changes:

1. Added setImmidiate() with example
2. Some other miscellaneous updates and refactors.
@Yash-Handa
Copy link
Contributor

Thanks, @fhemberger, @keywordnew, @Maledong, @ZYSzys, and @willin for such prompt suggestions, reviews, and merges 🙌🙌.

I would love to have some suggestions about the below 3 articles:

  • how-to-use-the-tls-module

    • RELEVANT(starttls section needs re-writing)
    • The basic concepts are relevant but references to node’s tls module need re-writing
    • Note: I don’t have enough knowledge/experience with tls module to incorporate all best practices of it. Any assistance from a security/crypto expert would be highly appreciated.
  • how-to-create-default-parameters-for-functions

    • IRRELEVANT(it’s in ES6 specification)
    • The article is all about workarounds(hacks) to use default parameter like feature in ES5 and below.
    • Suggestions: This stack overflow answer is all that we need.
  • using-ECMA5-in-nodejs

    • IRRELEVANT(Similar doc exists in nodejs.org website)
    • This Article covers ECMA5 but a general article already exists in nodejs.org website here
    • node.green is another great source to use.

@fhemberger
Copy link
Contributor Author

a) I think we can safely remove "using-ECMA5-in-nodejs".

b) "how-to-create-default-parameters-for-functions": ES6 default parameters should be the default, also the part of "optional values in the middle" is still useful. Ditch everything after "More complicated cases require more code …"

Can't say much about TLS either ¯\__(ツ)_/¯

@Yash-Handa
Copy link
Contributor

Thanks, @fhemberger for your suggestion as always, 😄

@jonchurch
Copy link
Contributor

Hey @Yash-Handa, do you have a list of the articles you are currently working on?

I want to help with this task, and want to make sure we don't overlap efforts.

@Yash-Handa
Copy link
Contributor

Hey, @jonchurch great to know that 🎉
I have already answered a part of your Question here

For the list part:
I have just finished my initial 10 articles and didn't actually shortlisted the next batch of Articles. I suggest you select first (any number of articles you comfortable with) and do brief us about them on this very issue. Afterward, I will select 🙂

ghost pushed a commit that referenced this issue Mar 13, 2019
In accordance with issue #1977, I have updated the Article how-to-create-default-parameters-for-functions with the following changes:

1. Added modern JavaScript (ES6) default parameter specification.
2. Added "falsy" value anomaly in default parameter.
3. Removed an extra example for ES5 and below.
4. Some other miscellaneous updates and refactors.
@jonchurch
Copy link
Contributor

Here is the list of articles I intend to start working on. I can commit to these for now and will come back for more soon 👍.

All of these could benefit from basic formatting improvements.

REPL

  • how-to-use-nodejs-repl

    • Still relevant. Introduces the REPL.
    • This article is very light introduction to the existence of the node REPL.
    • Suggestions: Add the REPL special commands list, mention how to exit repl, mention example of when you might want to use the repl.
  • how-to-create-a-custom-repl

    • Still relevant, just needs some formatting improvments
    • Suggestions: Touch up formatting, refactor for clarity.

Advanced

ghost pushed a commit that referenced this issue Mar 14, 2019
In accordance with issue #1977, I have updated the Article how-to-access-query-string-parameters with the following changes:

1. Added a note on how to execute the program.
2. Added some detail about other key values provided by `url.parse()`.
3. Some other miscellaneous updates and refactors.
ZYSzys pushed a commit that referenced this issue Mar 26, 2019
Updates to knowledge article **[how to create a custom repl](https://github.com/nodejs/nodejs.org/blob/master/locale/en/knowledge/REPL/how-to-use-nodejs-repl.md)**.

Per #1977 (comment).

This one didn't need much in my opinion. I updated the code block formatting and tweaked some phrasing. I did not add section headers to this one.

* Update code fence syntax
* Replace first person use of "I" with "you"

cc @keywordnew @fhemberger 

Co-authored-by:  <maledong_github@outlook.com>
@fhemberger
Copy link
Contributor Author

/cc @amiller-gh @nodejs/website-redesign We are updating the knowledge base right now, there's a lot of good content that should find its place on the new website as well.

apal21 added a commit to apal21/nodejs.org that referenced this issue Apr 24, 2019
In accordance with issue nodejs#1977, I have updated the Article how-to-access-query-string-parameters with the following changes:

1. Used const instead of var.
2. Removed unnecessary server variable.
3. Added a note on how to execute the program.
ZYSzys pushed a commit that referenced this issue Apr 24, 2019
In accordance with issue #1977, I have updated the Article how-to-access-query-string-parameters with the following changes:

1. Used const instead of var.
2. Removed unnecessary server variable.
3. Added a note on how to execute the program.
apal21 added a commit to apal21/nodejs.org that referenced this issue Apr 25, 2019
In accordance with issue nodejs#1977, I have updated the Article how-to-create-a-HTTP-server.md with the following changes:

1. Used `const` instead of `var`.
2. Added CURL example.
ghost pushed a commit that referenced this issue Apr 27, 2019
In accordance with issue #1977, I have updated the Article how-to-create-a-HTTP-server.md with the following changes:

1. Used `const` instead of `var`.
2. Added CURL example.
@alexandrtovmach
Copy link
Contributor

This issue should be closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content Issues/pr concerning content help wanted
Projects
None yet
Development

No branches or pull requests

5 participants