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

Update linter version and still observe max line length #355

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

Conversation

justin808
Copy link
Member

@justin808 justin808 commented Aug 13, 2018

Yes, need to remove local path for the linter...

Questions:

  1. What's the right prettier setup?
  2. Should we put in the rule of max-len

This change is Reviewable

Yes, need to remove local path for the linter...

Questions:
1. What's the right prettier setup?
2. Should we put in the rule of max-len
Copy link
Member Author

@justin808 justin808 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 6 files reviewed, 4 unresolved discussions (waiting on @alexfedoseev)


.eslintrc, line 5 at r1 (raw file):

  - eslint-config-shakacode
  - prettier
# Not sure if below is better Alex

@alexfedoseev ?


.eslintrc, line 26 at r1 (raw file):

  # https://github.com/benmosher/eslint-plugin-import/issues/340
  import/no-extraneous-dependencies: 0

@alexfedoseev Thoughts?


package.json, line 12 at r1 (raw file):

    "build": "babel --out-dir lib src",
    "lint": "eslint --ext '*.js,*.test.js'  .",
    "prettier-eslint": "prettier-eslint --ext '*.js,*.test.js'  .",

@alexfedoseev should we use this one?


examples/multiple-entries/webpack.dev.config.js, line 14 at r1 (raw file):

      'font-awesome-loader',
      `bootstrap-loader/lib/bootstrap.loader?configFilePath=` +
        `${__dirname}/bs3.yml!bootstrap-loader/no-op.js`,

in this case, a long line is easier to read, but there are too many cases where this is not the case and long lines are not caught by the linter

Copy link
Member

@alex35mil alex35mil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't used any of those for quite awhile so my review is not that valuable at this point.

Reviewed 6 of 6 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions

@ahangarha
Copy link
Contributor

@justin808 What is blocking this PR?

@ahangarha
Copy link
Contributor

@justin808 The objective is not clear to me.
May you clarify what we want to achieve?

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

Successfully merging this pull request may close these issues.

3 participants