Skip to content
This repository has been archived by the owner on Apr 11, 2019. It is now read-only.

Remove linting modifications #96

Open
armenzg opened this issue Jan 29, 2018 · 14 comments
Open

Remove linting modifications #96

armenzg opened this issue Jan 29, 2018 · 14 comments

Comments

@armenzg
Copy link
Contributor

armenzg commented Jan 29, 2018

I would like us to remove all linting modifications from .eslintrc.js
We should make our project be as close as possible to the airbnb linting rules.

This increases consistency across projects.

@marco-c
Copy link
Collaborator

marco-c commented Jan 29, 2018

Have you considered using the Mozilla rules (eslint-plugin-mozilla) instead?

@armenzg
Copy link
Contributor Author

armenzg commented Jan 29, 2018

That seems rather oriented for the Firefox build environment.
@eliperelman and I are focusing on using AirBnb's linting rules accross our various web projects.

@cuniculture cuniculture self-assigned this Jan 31, 2018
@cuniculture
Copy link

The default Airbnb .eslintrc.js is pretty empty. Is there another file you are already using as a guideline? Or are you referencing the various 'eslint' tags in the Airbnb React Style Guide.

@armenzg
Copy link
Contributor Author

armenzg commented Jan 31, 2018

This is the one we're using:
https://www.npmjs.com/package/eslint-config-airbnb#eslint-config-airbnb-1
It extends from these four:
https://github.com/airbnb/javascript/blob/master/packages/eslint-config-airbnb/index.js
For instance, this is the rules that are specific to react:
https://github.com/airbnb/javascript/blob/master/packages/eslint-config-airbnb/rules/react.js

I think the guide is an explanation of what the rules enforce.

@sylvestre
Copy link

@ahal @Standard8 FYI, as you are the expert on fx side ;)

@Standard8
Copy link
Member

That seems rather oriented for the Firefox build environment.

Just a slight clarification - it is oriented towards Firefox development, not just the build environment. Having only glanced at airbnb's style guide, I think a lot of the rules are likely to be the same (except for quotes, they seem to prefer single, we seem to prefer double).

eslint-plugin-mozilla has a small subset of rules at the moment, I'd like to be adding lots more, it just takes time to roll them out unfortunately due to dealing with all the legacy issues. Note for dealing with actual code style, we're likely to look towards prettier in the near future - I know devtools and various other projects are using this outside m-c.

I'm OK with other projects using different ESLint rule sets, as long as they are never going to touch mozilla-central (as in be incorporated into m-c). For mozilla-central, we need to be as consistent as possible, to make it easier for developers touching any part of the code base.

Personally, I think maybe Mozilla as a whole should have one style, so that developers working on any project have consistency. Realistically I think that'd probably be quite hard to convince people to do, so for now I'm heading down the lets make m-c consistent route.

@eliperelman
Copy link

I'm OK with other projects using different ESLint rule sets, as long as they are never going to touch mozilla-central (as in be incorporated into m-c). For mozilla-central, we need to be as consistent as possible, to make it easier for developers touching any part of the code base.

I agree with this.

Realistically I think that'd probably be quite hard to convince people to do, so for now I'm heading down the lets make m-c consistent route.

This is an important distinction. It is tricky to re-use ESLint configurations for multiple targets, e.g. m-c vs. Node.js vs. the browser. I would agree that particular environments should warrant their own useful, but specialized, ESLint configurations.

@cuniculture
Copy link

@Standard8 et all, I'm new here, and don't have a lot of context for this project yet. Do you see any situation where firefox-code-coverage will be included in Mozilla Central?

@eliperelman
Copy link

I don't believe so, this is meant to be running on an external server, and to be updated/deployed continuously.

@armenzg
Copy link
Contributor Author

armenzg commented Feb 1, 2018

@BeaverValley in other words, we can continue with the original plan.

Removing the added rules in .eslintrc.js and fixing the linting issues will be sufficient.

Thanks!

@armenzg
Copy link
Contributor Author

armenzg commented Mar 29, 2018

@BeaverValley I will be away from tomorrow until May 7th.

Please join #frontend-infra on IRC to talk with other people if you would like other React related contribution opportunities.

Take care!

@armenzg
Copy link
Contributor Author

armenzg commented May 26, 2018

Hi @BeaverValley please let me know if have a fix for this, otherwise, I will put it back into the pool of available issues for contributors by this Monday.

@akansha2608
Copy link
Contributor

Hi @armenzg , @marco-c can i take this up as my first bug

@marco-c
Copy link
Collaborator

marco-c commented Sep 26, 2018

Sure!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants