Skip to content

Fixes #30, Add ESLint to project#36

Closed
Vishwajeetsinh98 wants to merge 1 commit intosavoirfairelinux:masterfrom
Vishwajeetsinh98:master
Closed

Fixes #30, Add ESLint to project#36
Vishwajeetsinh98 wants to merge 1 commit intosavoirfairelinux:masterfrom
Vishwajeetsinh98:master

Conversation

@Vishwajeetsinh98
Copy link
Contributor

Fixes #30

Fixes #34

Changes proposed in this pull request:

  • Add .eslintrc.json to project with eslint rules.

Status

  • READY
  • HOLD
  • WIP (Work-In-Progress)

How to verify this change

eslint filename

Additional notes

If so desired, a pre-commit hook change the package.json like so:

"scripts": {
    "lint": "eslint ./ --cache --ignore-pattern .gitignore"
  }, 
"pre-commit": ["lint" ]```

@ssirois
Copy link
Contributor

ssirois commented Oct 7, 2017

Welcome to Ringme.js @Vishwajeetsinh98 and thank you for this contribution.

Sadly enough, this issue has already been tackled by @DSchau in #32

Feel free to check the updated trunk and propose tweaks on ESLint. Be aware that there is also #34 by @nabrahamson in progress.

I would really appreciate if you could transform this PR into a specific pre-commit hook PR. That would be great. Don't forget to document the pre-commit hook inside CONTRIBUTING.md.

@ventilooo
Copy link
Contributor

@Vishwajeetsinh98 ping ? could you rebase your work ?

@Vishwajeetsinh98
Copy link
Contributor Author

@ventilooo I don't really think a rebase is required. All I've changed are dependencies and the package.json file.

@Vishwajeetsinh98
Copy link
Contributor Author

@ventilooo I think pulling will be safe.

@ssirois
Copy link
Contributor

ssirois commented Oct 19, 2017

@Vishwajeetsinh98 OK. I'll take a look and merge this manually.

Copy link
Contributor

@ssirois ssirois left a comment

Choose a reason for hiding this comment

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

I did a manual merge and made sure that npm run lint was still running fine. See commit a642087

@ventilooo ventilooo closed this Oct 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants