Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#18 Move to ES6 #3

Merged
merged 3 commits into from Feb 12, 2017

Conversation

Projects
None yet
2 participants
@frsv
Copy link
Contributor

commented Feb 10, 2017

I've added some ES6 specific rules in addition to esling-plugin-node rules. I'm not sure if they are really necessary, so maybe, should I exclude it from config?
I guest, the most important rules are:

'node/no-unsupported-features': 'error',
'node/no-deprecated-api': 'error',
'no-const-assign': 'error',
'no-var': 'error',
@ai

This comment has been minimized.

Copy link
Member

commented Feb 11, 2017

  1. How no-const-assign with node/no-unsupported-features? Safari doesn’t support const outside strict block. Adding strict mode comment for all files is not a best idea (especially, because const is just a trend, it doesn’t help really).
  2. Maybe we should force to use => function too?
  3. You could extend index in node4. It reduce mistales when you add/remove rule to one config, but forgot about other.
  4. ES5 and ES2015+ are not clear. We should describe when you should use one, and where another. Maybe For server-only project and For browser and universal project?
@frsv

This comment has been minimized.

Copy link
Contributor Author

commented Feb 11, 2017

  1. It works with
"engines": {
    "node": ">=4.0.0"
},

in package.json, but I didn't know this Safari behaviour. Should we use let instead or remove 'no-var' rule with 'no-const-assign' rules?
2. Good idea! In my opinion, it's better to use them with parens and body always
3. Do you know any way to disable all plugin rules inside extendable config file? I don't, so I added base.js, which contains all basic rules for both config files.
4. Ok, got it, good point.

@ai

This comment has been minimized.

Copy link
Member

commented Feb 12, 2017

  1. Oops, seems like I was very tired yesterday. Why we need to care about Safari for node.js config :D.
  2. I like parens-less => in map and other FP methods :D.
  3. You can write "plugin/name": "off" to disable rule from extended config. You need to specify all rules manually, but I think it is less problem.
@ai

This comment has been minimized.

Copy link
Member

commented Feb 12, 2017

Also I will need your Twitter and VK name for announce

@frsv

This comment has been minimized.

Copy link
Contributor Author

commented Feb 12, 2017

  1. Actually, it doesn't matter to me – changed this rule
  2. Well, I think it's not the best idea, since we have to watch for new rules added to plugin:es5/no-es2015 and plugin:es5/no-es2016 configs and disable them in our node4.js file. My approach completely split rules for ES5 and ES2015 – configs don't know of each other at all.

My twitter username is @cyber_blyat and VK link is https://vk.com/b3ard0m

@ai ai merged commit ff1fc14 into logux:master Feb 12, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ai

This comment has been minimized.

Copy link
Member

commented Feb 12, 2017

Creating a base.js was great idea.

@ai

This comment has been minimized.

Copy link
Member

commented Feb 12, 2017

I also renamed index.js to browser.js 65e8a43

@ai

This comment has been minimized.

Copy link
Member

commented Feb 12, 2017

I need to go eat right now. You can continue work with logux-server by using unreleased version:

"eslint-config-logux": "logux/eslint-config-logux",

Also, don’t forget to use Yarn, instead of npm (Travis CI uses yarn.lock so you will have broken CI when you will start work with logux-server).

Great work, I will release version when we finish updating server (to be sure, that everything is OK).

@lamartire lamartire referenced this pull request Feb 13, 2017

Closed

Remove strict rule #6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.