search

MetaMask / metamask-mobile

Mobile web browser providing access to websites that use the Ethereum blockchain
https://metamask.io
Other
2.15k stars 1.11k forks source link

Extend the shared MetaMask ESLint config #2551

Open rekmarks opened 3 years ago

rekmarks commented 3 years ago

Mobile should extend:

As discussed, rules can be disabled in the local ESLint config to make the overhead of migrating smaller.

For related work, see: https://github.com/MetaMask/eslint-config/issues/170, https://github.com/MetaMask/eslint-config/issues/171

This is a major change for dev experience (has no impact on users). After this change takes place, the engineers will have to code in a different way (before / after moment)

rekmarks commented 3 years ago

When mobile does this, it's also a significant opportunity to get on the same Prettier config (although that change would make sense to do in a separate PR). If the mobile team feels strongly about its Prettier config, we should discuss what can be done to reconcile our differences of opinion.

Personally, I find 120 print width far too much.

andrepimenta commented 3 years ago

I believe this is an important step to share more code and configurations with both products.

Context: https://consensys.slack.com/archives/G1L7H42BT/p1618866302118900

Started looking into this here: https://github.com/MetaMask/metamask-mobile/compare/improvement/shared-metamask-eslint-config

mobularay commented 3 years ago

High impact for Engineers - need to align on timing as it changes the way code works. Engineers to discuss before working on this workl.