30-seconds-of-code: Enforcing a coding standard

Created on 12 Dec 2017  路  25Comments  路  Source: 30-seconds/30-seconds-of-code

I think that enforcing a coding standard would benefit this project by bringing consistency and readability of the project.

I believe using a syntax enforcer such as XO or even custom ESLint settings would bring consistency to the scripts before pull requests. Another option would simply be to use a syntax formatted such as Prettier to format all code when building. It would also help readability since newlines etc. would be formalised.

For example, var is usually discouraged when writing ES6. When only one parameter is used in an arrow function, it is sometimes recommended to be written as x => f(x) and not (x) => f(x) etc.

The simplest option is using a formatter, since it would only need a change in builder.js and won鈥檛 require contributors to follow the wanted format. XO or ESLint however would rather aid the contributors to follow a certain format and perhaps teach them a thing or two about following a coding standard.

Are these thoughts solely my own or is this something that others think would help the project?

enhancement help wanted

Most helpful comment

Guys I am not saying that we should forget pure vanilla js approach, but thing is this repo is mainly for few liner snippet so the methods like Map, reduce, filter, sort, etc. makes more precise and readable. It would be beneficial when someone is refactoring their codebase i.e. mostly bloated and will use these type of methods.

All 25 comments

I also feel that the project as a whole would benefit from something like this. I am not particularly sure how to get the formatter up and running, but I'll check it out. In the meantime, I will try to write a rough sketch of the guidelines in CONTRIBUTING.md, so that contributors can read through them before submitting PRs.

Right now the amount of PRs is pretty low, so I can polish them up myself (along with existing snippets). Hopefully, future PRs will follow the styleguide that will be written soon.

Oh and using var is just one of my bad habits, I'll change these to const as soon as I get a chance.

Yes right said @Chalarangelo .

  • Main focus will be on arrow functions, const keyword and using spread operators(...). (ES6 approach)

  • Likewise use .map() instead of forEach(). Promote uses of methods like sort(), filter(), etc...

I think Prettier is the way to go. That way you can pretty much add a step in the builder where you extract the code from the readme, run Prettier on it and put it in the finalised output. It would take care of everything without you needing to take time to specify any rules. You could even add a step to format each pull request automatically if needed.

Prettier is used mainly for project based repos. Like for more greater codebase.

Here we only describe one liner or few liner snippet. So it would be unsuitable to use for smaller format code. Atlast it's everyone's decision.

@AlexGustafsson after I deal with the PRs that are currently pending and a few other things, I will write proper guidelines for contributors, as they are necessary. When I get the time, I will look into Prettier, as it sounds like a good tool to automate the process, but I have no experience with it whatsoever. If anyone has the know-how and time to get it started, I will gladly maintain it, but right now I barely have the time to finish up the PRs and what the community has asked in the last day. Hopefully, things will quiet down later and I will be able to work on this.

@Chalarangelo Maybe we could use ESLint. ESLint with configuration from Airbnb for example. Here is their style guide: airbnb javascript style guide

It would keep code style consistent within the whole project.

@fejes713 I will dive in and see what I can do, I like this idea, as the Airbnb style guide is pretty good.

@Chalarangelo If the help is needed I am more than ready to collaborate. I've played with ESLint last few weeks and it works perfectly. Good thing is that you can also override the Airbnb's style guide to your needs.

@fejes713 I would love some help with this, mainly:

  • Update the builder script to figure out where the snippet is and send it to ESLint.
  • Make sure the snippet that comes out runs properly (we have to set up testing now, don't we?).
  • Give me the gist of how to configure ESLint to do my/our bidding, so that we can setup proper guidelines.

I will get started on the styleguide etc. and update CONTRIBUTING.md in the next few hours (I have to add samples for each functions first, as requested in #24) and then I will write a short and clear set of guidelines on how snippets should be formatted, so we can setup the rules in ESLint.

The only problem I see with this is that everyone is submitting .md files. As far as I know, ESLint does not work with .md files. ESLint checks JavaScript code based on given rules.

In order to install it:
$ npm install -g eslint
then run the following from working directory:
eslint --init
It will create .eslintrc file for you where you can specify the rules you want to use. The rules can be found here: docs

You can also then add "extends": "airbnb-base" to the .eslintrc file to make it use Airbnb's style guide.

I suppose I should write a quick and dirty extractor package to grab the js snippet from the markdown files and pop it into ESLint.

@Chalarangelo No need really, it鈥檚 basically a one liner extracting content between the first and last occurance of three ticks (`)

@AlexGustafsson Indeed, I have a tendency to onverthink things.

Won't ESLint slow down the build process significantly if we have to reevaluate every snippet individually? Also, can it be called from the build script with some text input and return the output? Examples appreciated, if anyone knows how to do this.

@Chalarangelo your comment reminded me of this project: https://github.com/Rich-Harris/lit-node which could be interesting when used with this guide!

@antony Interesting, I'll check it out!

@meetzaveri why do you say to use .map() instead of .forEach()? Both of them have very different use cases and performance

@skatcat31 Here there will be more use of Array data structure. So likely the devs will use map() before forEach(). This might not be a good argument but makes sense when application is few liner snippets.

@meetzaveri I second this. It鈥檚 easy to forget about the functional aspect of JavaScript and roll with for-loops. Map, reduce, filter etc. makes the code more readable and displays a better use of modern language features than forEach in this particular case.

Array.forEach is a function? Or am I missing what you're trying to present in your response?

For loops should be avoided I agree. They should only be used in functions that would want such behavior. Most of these will not be those.

Guys I am not saying that we should forget pure vanilla js approach, but thing is this repo is mainly for few liner snippet so the methods like Map, reduce, filter, sort, etc. makes more precise and readable. It would be beneficial when someone is refactoring their codebase i.e. mostly bloated and will use these type of methods.

@meetzaveri I'm guessing I missed something. So for calrity, are you talking about:

[1,2].map(console.log) // use this
[1,2].forEach(console.log) // over this

or

// avoid this
var a = [1,2];
for(let i = 0; i < a.length; i++){ console.log(a[i]) } // including for...in variant

If it's the latter I totally agree. If it's the former, forEach came about very close to the same time as .map IIRC( NodeJS first supported them both in the 4 branch[ignoring odd versions]), and both have different use cases and scenarios. It may be best to highlight the fact that in a functional capable langauge using the right tool for the right job is a good practice since not all applications can use general use cases or care about throwing away a result(will rarely come up).

Really this is just for my own clarificaiton, I'm not saying it's wrong, I'm just curious if it's because in most cases it would never matter(I come from a world where it can very greatly matter, and so I'm purely curious)

@skatcat31 Good catch though and also came to know this fact map() IIRC( NodeJS first supported them both in the 4 branch[ignoring odd versions]) .

Hey guys! Please join us on gitter to discuss this further, before I start writing the guidelines.

I just updated CONTRIBUTING.md with some easy-to-follow guidelines for contributors and a modified and condensed styleguide, based on the Airbnb Javascript Styleguide. I'll close the issue, but feel free to comment, open PRs etc.

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for any follow-up tasks.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

konglx90 picture konglx90  路  6Comments

Priyansh2001here picture Priyansh2001here  路  5Comments

fejes713 picture fejes713  路  4Comments

kingdavidmartins picture kingdavidmartins  路  5Comments

YusofBandar picture YusofBandar  路  5Comments