Now what?

[SomeoneWeird]

ESLint supports hundreds of extra rules - it supports things like making sure Promises have .catch handlers, so you don't accidentally forget! Standard doesn't have these enabled.

Let's change that.

If you want to suggest a new eslint rule - open an issue for it. Add the name of the rule, and why you think it should be added. People can then 👍 / 👎 and after a week(?) or so, we can merge it in?

I'm going to start with @obrin and open a few issues with really useful rules that we should have added, but they are by no means locked down, if you have issues with one of them, speak up!

[obrin]

We should probably decide on a minimum number of +1s

[mazzy89]

We would suggest airbnb eslint rules with internal iflix customization according our needs

[obrin]

yeah, we can use that as a reference but this won't be based off airbnb but standard instead

[timmyw]

Some of the things being suggested are very good guidelines, but seem quite restrictive if they are enforced. Should we come up with list of "good to follow" and "must be followed" items?

[SomeoneWeird]

@timmyw I'm not sure it's a good idea - for the same reason that standard doesn't let you change any of it's rules .

[mazzy89]

@obrin ok so let start from this https://github.com/xjamundx/eslint-plugin-standard and create a doc like airbnb introducing our rules. @timmyw @obrin already opened some issues about that

[timmyw]

@SomeoneWeird I understand the slippery slope of letting some of these be guidelines instead of rules, but, as an example forcing people to not use string concatenation across the board seems draconian. There are probably some items which can be left to the better judgement of the people coding them.

[mazzy89]

@timmyw I think that we need to advocate as much as possible the new ES6/ES7 feature in order to have a codebase consistent over the entire organization. Which should be the use case in which concatenation is better than string interpolation?

[timmyw]

@mazzy89 not sure of use cases off the top of my head, but I would imagine string concatenation is probably more efficient than the use of templates. Do we functionally gain anything from enforcing the use of templates? If not, there may be situations where the programmer feels one is better than the other, pretty sure those can be argued out in PR discussions. I don't want to get bogged down in a single item - I just feel that some things should be left to the human - we already have some draconian "look and feel" things imposed by standard, I don't really want to add to them.

[mazzy89]

@timmyw now I completely see your point of view. probably if we want to discuss more about for example string interpolation we should take in account the case of frontend code vs backend code. String interpolation might be sometime annoying in some FE framework while in the backend I've found it very useful and definitely much better than string concatenation.

[timmyw]

@mazzy89 - so sometimes it is a good idea and sometimes not... and the coder should decide. My standpoint is that tools like standard should be enforcing functional errors (or potential errors), not stylistic/layout issues. (Like a space between function name and parenthesis, unless you're calling the function, no blank line before a closing brace - things that may (subjectively) make something more readable, but don't affect the functioning of the code in any way.

[mazzy89]

well @timmyw on that I don't agree with you. these tools have to enforce spaces, parentheses and everything that affect the style/layout of the code but maybe this is my opinion because I'm a great supporter of tools which enforce consistent in all the layers. in the last two years tools like fmt from golang have been widely used and accepted by the entire community in the golang ecosystem because enforce to look at the code in the same way and you know less time you look at the code more you are productive to understand and master the code. but again this is my opinion. ;-)

[mazzy89]

for example I know that both these piece of codes will work:

if (isOpened) {
  console.log('is opened')
}

if(isOpened)
{
console.log('is opened')
}

personally with the second I'm freaking out 👎

[brian-lim]

i'm used to the 1st as well... but use to writing the 2nd with some spaces...

[uwaseem]

@timmyw I personally feel that certain stylistic/layout issues should be enforced as well. This is because many programmers will be working on one repo. If everyone have their own styles, the code would just become messy. This will then impact productivity as programmers would need to understand multiple standards or styles instead of one that is agreed by the majority.

[SomeoneWeird]

@timmyw

but I would imagine string concatenation is probably more efficient than the use of templates

They both compile down to the same thing, so doesn't really matter

[SomeoneWeird]

@mazzy89 Absolutely - there are definitely some JS coding conventions we won't stray from, { on the previous line is one of them :p

[obrin]

Lets get some 👍 or 👎 on some of the issues to keep the ball rolling? I only raised a few to start out; so it would be great if everyone else could suggest more. We'll leave it open for awhile and see how it goes.

[ronny]

Say we start using this version of standard for our new services. We need to go through each project, update it to use standard, run standard (or whatever npm script for it), fix (or standard --fix?) every violation, PR it and get it merged, move on to the next project. This would probably take days if we want to be nice to people with significant changes in their branches and coordinate with them.

Now, say in the future we want to add or change some rules. We PR it here, get it merged, and then repeat the above process again?! I guess we could lock the dependency to a specific version of our standard, i.e. use exact version number instead of ^ or ~. In fact, I think we should, otherwise we'd potentially break builds. We'd still need to repeat the steps above, but this will allow us to upgrade each project over time rather than at once.

[SomeoneWeird]

@ronny Sorry, for some reason I missed this notification. Yes - if we change rules that would break existing projects (most probably would?) then that would have to be a semver major bump in the standard (we need another name) library, so everything is an explicit bump.

[joshgillies]

This shouldn't come as a surprise but personally I'm not in favour of maintaining/supporting a fork of standard. 😕

Have we given any thought toward simply using one of the following: http://standardjs.com/awesome.html#forks?

[SomeoneWeird]

@joshgillies why aren't you in favour? :)

[joshgillies]

I guess I didn't get my message across at xtra... 😭

But mainly, I'm not convinced it's a good spend of our time. If tweaking standard eslint values is something you feel strongly about, push this discussion with upstream?

[SomeoneWeird]

Potentially! It's a tradeoff. Once we have these rules in place, we won't have to maintain them anymore though. Feross has explicitly said that he's not adding rules, and that standard is his™. There are hundreds of eslint rules that can actually be beneficial to us as a team (not syntax styling ones, but stuff like http://eslint.org/docs/rules/eqeqeq) that will prevent problems in the first place.

[joshgillies]

OK sure, but what about new rules that will come as a result of an evolving language... This will be an ongoing discussion at the very least.

[SomeoneWeird]

For sure - if we want. By that argument we're spending time learning new things in our language. Why do we do that? Because it's cool, might make it easier for us to write code etc? We already spend time upgrading our node versions, upgrading modules. If we now spend another little bit of time to make sure the tooling around our language works for us the best, is that bad use of our time?

[tjstebbing]

Consensus would be .. 50% of our current engineering team voting +1, not 4-5 people.

[SomeoneWeird]

Who ever said anything about consensus? These issues have been open for nearly a month - there have been a multitude of @ message on slack, and a number of emails that have told everyone about them. People have had enough time to vote on them if they don't think we should do so. If somebody brings up valid reason of why we should not enable the rule, we're definitely not in the business of ignoring them.

[tjstebbing]

Ok, well, people are busy working on their work, yet now we have to stop to have this argument again. I did not want to adopt Standard at all, but now it's here we should stick to it.

The primary argument for bringing it in in the first place, that it avoids arguments about linting, is being broken by starting discussions about changing our linting Standard for Iflix.

Here is my 'valid reason we should not enable' any more rules: This is wasting everyone's time, we already made the choice to go with Standard. If we're questioning the primary premise for bringing in Standard then I will voice my opinions all over again about the value (or lack of) in extreme linting.

[SomeoneWeird]

We're not removing standard, definitely not, what we're looking at doing is adding more rules to make it more effective and useful in our everyday workflow.

We know how javascript works, and what pitfalls to avoid and how. We're going to be an engineering team of 100+ soon, and 200+ next year. If we can codify these pitfalls to stop all of our other engineers falling into them then that's great. That will save the company both time and money - yes, you might lose some time yourself (but if you can't find a couple hours every month or two to put towards tooling then we might have larger issues?) but in the long run you'll save everyone time.

[obrin]

@pomke

A) Here’s what we seem to agree on 1- Linters are meant to catch bugs 2 - Arguing on what to lint over and over again is a waste of time for the most part. I’ve got better things to do

B) Here’s what we don’t seem to agree on 1 - Linters should standardise the format and how we write code

Addressing these points B1) This might seem fine when working with one or two people. But what happens when we scale to hundred or engineers that write javascript differently? You might or might not agree with this but I believe reading a book with one handwriting/font is easier then reading a book with hundreds of styles.

A1) Standard does not currently solve this problem well enough. Adam has given a couple of examples A2) I hate this bit just as much as other people do. Which is why we went with standard in the first place, but it doesn’t catch a number of bugs. Which is why we need our own fork of standard. Once this has settled, the argument of rules should not keep popping up.

Just to summarise why we need to fork standard

1. Catch bugs
2. Save heaps of accumulated time on PR
3. Easier to read a codebase with a standard format
4. Stop arguing about the same rules over and over again. Just discuss about it once and move on
5. We work as a team :)

[timmyw]

@obrin: Agree on A1 + A2 On B1 - we're all adults. Make it policy to follow the existing style of the project you're working on. Make it policy to favour code correctness over where or where not a newline should be.

[tjstebbing]

Teaching people to appropriately comment their code would have 10x the impact of linting rules for code readability. I Don't buy into the argument that slightly different style s of code impede anyone reading the code ever. Show some evidence that it does? If people appropriately commented their code with 'why' not 'what' style comments you would get the readability improvement you are looking for, without the downsides I've listed in previous posts to do with aggressive style linting.

[SomeoneWeird]

@obrin 💯 @timmyw 💯

@pomke Absolutely - but why not both? If we can make the tools that we already use everyday do even more for us, we would be stupid not to?

[obrin]

@timmyw @pomke yes definitely agree on that.

On B1 - we're all adults. Make it policy to follow the existing style of the project you're working on. Make it policy to favour code correctness over where or where not a newline should be.

Yes definitely, but if that policy can be enforced then there's no need to always keep an eye for people who are not feeling like adults :) Before enforcing any linters we've had repositories with 4 spaces and 2 spaces everywhere.

@pomke you might or might not find 4 spaces and 2 spaces mixed in the same function annoying but it blows my mind :) If you feel the same way then there shouldn't be a problem enforcing the same styles :) With linters or without, anyone contributing to a repository should follow it's format (not a right place to be a bandit). with linters it's just possible to enforce a certain aspect of this.

[timmyw]

@obrin - yes definitely we are all adults, but let's enforce whitespace/layout guidelines?

Does 4 spaces/2 spaces really prevent you from reading the code?

[SomeoneWeird]

@timmyw No, it doesn't matter which one we use (imo, others might have differing opinions) but it should be the same across everything.

[obrin]

Yeah we should have guidelines, but why not reflect it in the linter as well? AirBnB has an phenomenal guideline which definitely took lots of effort; it's also reflected in their rules. I don't see why not have them hand in hand.

Does 4 spaces/2 spaces really prevent you from reading the code?

No it doesn't, but should we be mixing it up? where's the consistency?

If we don't enforce this, it would mean that we won't enforce rules such as semicolons or spaces/tabs. And then we go back into that death spiral again :) It's just really annoying. We just need to agree and move on :)

[brian-lim]

I would like a consistent 4 or 2 spaces... not mix of both. Not because we do not understand the code written, but because it will be hard to skim thru the code and checking the logics.