Skip to content
This repository has been archived by the owner on Jan 10, 2022. It is now read-only.

WIP: remove .eslintrc, use eslint-config-nrf #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

IvanSanchez
Copy link
Contributor

This has been prompted by NordicSemiconductor/pc-nrfconnect-launcher#186 (comment) - if the license header is a requirement, why aren't we managing that automatically? Also, why are we copy-pasting .eslintrc files between projects?

This PR is an attempt to provide a solution for both, in the form of a small monorepo and scoped npm packages (to no pollute the @NordicSemiconductor scope at this point). IMO this approach adds the bit of flexibility that nrfconnect-devdep lacks, and is the minimum amount of configuration needed.

@bencefr
Copy link
Contributor

bencefr commented Mar 23, 2018

Unfortunately eslint-plugin-notice doesn't seem to be able to match its own template.

@mrodem
Copy link
Contributor

mrodem commented Mar 26, 2018

Great improvements! I really like the automatic license insertion on eslint --fix.

The following rule in eslint-config-nrf-react could maybe be moved to an eslint-config-nrf-nrfconnectapp package, as it is only needed for nRF Connect apps:

        "import/no-unresolved": [
            "error",
            {
                "ignore": [ "nrfconnect/.*", "pc-ble-driver-js", "pc-nrfjprog-js", "serialport", "electron" ]
            }
        ],

In such a package, we could maybe also disable react/prop-types only for the index.jsx using a file pattern. This is something we have in the boilerplate's index.jsx now. There are probably some other rules we could add to aid app development.

I see that there are also some duplicate rules in eslint-config-nrf-base and eslint-config-nrf-react. If the intention is that we always want to extend the nrf-base rules, then I believe the duplicates can be removed from the nrf-react package.

@IvanSanchez
Copy link
Contributor Author

So shall we create a nordicsemi/eslint-config-nrf repo and take it from there?

@bencefr
Copy link
Contributor

bencefr commented Apr 3, 2018

I think it's a good idea to take the control and set our rules, I agree to have a repo for this.
For the eslint-plugin-notice I'd suggest to have a looser check maybe only for the first line, since it's broken as it is now.

@kenr
Copy link
Contributor

kenr commented Apr 3, 2018

This is a good idea.

An observation:
nrf-device-lister-js: eslint ^4.16.0
pc-nrfconnect-dev: eslint 3.19.0

Since the eslint configuration file format has changed between v3 and v4 and nrf-device-lister-js will be used from nrfconnect-connect-core which pulls in pc-nrfconnect-devdep, which use v3, I guess this PR will require some work in multiple projects?

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

Successfully merging this pull request may close these issues.

4 participants