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

Updating documentation about linters. #981

Merged
merged 3 commits into from
Mar 10, 2017
Merged

Updating documentation about linters. #981

merged 3 commits into from
Mar 10, 2017

Conversation

sweeneydavidj
Copy link
Contributor

Fixes: Consolidation and updating of documentation about linters used in the project.

Changes proposed in this pull request:

  • Removed outdated linter information , about SCSS, from main README
  • Removed outdated information about Ember Suave etc from top of CONTRIBUTING guide.
  • Added to existing documentation in Linter section of CONTRIBUTING guide - including external links to documentation about eslint-plugin-ember-suave and ember-template-lint.

cc @HospitalRun/core-maintainers

@@ -124,8 +124,9 @@ In addition, linters can help to enforce coding standards, find unused variables

**HospitalRun** uses the following linters:

1. [ESLint](http://eslint.org/) for ECMAScript/JavaScript. You can find the ESLint User guide [here](http://eslint.org/docs/user-guide/).
1. [ESLint](http://eslint.org/) for ECMAScript/JavaScript. You can find the ESLint User guide [here](http://eslint.org/docs/user-guide/). In addition to the default set of rules ESLint is setup to use eslint-plugin-ember-suave you can find more information about that [here](https://github.com/DockYard/eslint-plugin-ember-suave).
Copy link
Member

Choose a reason for hiding this comment

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

We need a link here to Dockyard's Ember.js style guide:
https://github.com/DockYard/styleguides/blob/master/engineering/ember.md

This has more helpful information than the eslint-plugin-ember-suave page.

Copy link
Member

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @sweeneydavidj!
As noted below, we need to emphasize that this project adheres to Dockyard's Ember.js Style Guide and link to that information because that will be helpful for developers trying to understand the rules used by ESLint.

@@ -124,8 +124,9 @@ In addition, linters can help to enforce coding standards, find unused variables

**HospitalRun** uses the following linters:

1. [ESLint](http://eslint.org/) for ECMAScript/JavaScript. You can find the ESLint User guide [here](http://eslint.org/docs/user-guide/).
1. [ESLint](http://eslint.org/) for ECMAScript/JavaScript. You can find the ESLint User guide [here](http://eslint.org/docs/user-guide/). In addition to the default set of rules ESLint is setup to use eslint-plugin-ember-suave you can find more information about that [here](https://github.com/DockYard/eslint-plugin-ember-suave).
2. [stylelint](http://stylelint.io/) for Stylesheets. You can find stylelint User guide [here](http://stylelint.io/user-guide/).
Copy link
Member

Choose a reason for hiding this comment

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

Can you correct the grammar here to You can find the stylelint user guide here

@sweeneydavidj
Copy link
Contributor Author

@jkleinsc thanks for the review! I've added in the changes that you mentioned.

Copy link
Member

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

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

Thanks for updating the PR. After seeing the changes, I think this statement belongs at the beginning of the document where it was instead of down in the linters section. Maybe clean it up to segue into the linters statement? Something like this?

This project uses the style guides from Dockyard for Ember and JavaScript. These style guides are enforced via ESLint, which is one of the linters...

@sweeneydavidj
Copy link
Contributor Author

@jkleinsc I've added in those changes, hopefully that reads better now.

Copy link
Member

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

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

Looks good to me @sweeneydavidj. Thanks for the PR! I'll merge it in.

@jkleinsc jkleinsc merged commit 0f06b3a into HospitalRun:master Mar 10, 2017
@sweeneydavidj sweeneydavidj deleted the update-linter-doc branch March 24, 2017 09:20
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.

2 participants