Skip to content

Conversation

@beamalsky
Copy link
Contributor

This updates our JS linter instructions to match the new Gatsby template currently in progress here: datamade/how-to#85

This PR needs updated links to the how-to repo before it can be merged.

An impeccable style guide, however, does not prevent human error. Every coder has at least one horror story about an hours-long hunt for a missing semicolon that caused the (sometimes silent) crash of an entire program. At DataMade, we recommend [ESLint](https://eslint.org/) as a way to enforce a coherent JS syntax and automate the hunt for typos.

## jshint
We integrate `ESLint` into our workflow as a step in continuous integration—typically, when any commit is pushed to a feature branch. This means that we catch syntax errors early, but are not stymied during local development as we introduce new code and troubleshoot. For an example of what this looks like, see the Gatsby app cookiecutter template. Notable files:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think I've ever set up ESLint in a Django project. Which is not great! Have you, @hancush? It would be good to link to a Django setup here as well.

Copy link
Member

Choose a reason for hiding this comment

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

I have not! Sounds like a good addition to our Django project template. Opened datamade/how-to#115.

Copy link
Member

@hancush hancush left a comment

Choose a reason for hiding this comment

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

Although it was never fully adopted, I liked that our fork of the Node style guide included discussion of why we preferred certain syntax. https://github.com/datamade/node-style-guide

Is there a similar document or discussion of the ESLint rules we're applying? It's not something I require to bring in these changes, but it would be good to include, if it exists!

An impeccable style guide, however, does not prevent human error. Every coder has at least one horror story about an hours-long hunt for a missing semicolon that caused the (sometimes silent) crash of an entire program. At DataMade, we recommend [ESLint](https://eslint.org/) as a way to enforce a coherent JS syntax and automate the hunt for typos.

## jshint
We integrate `ESLint` into our workflow as a step in continuous integration—typically, when any commit is pushed to a feature branch. This means that we catch syntax errors early, but are not stymied during local development as we introduce new code and troubleshoot. For an example of what this looks like, see the Gatsby app cookiecutter template. Notable files:
Copy link
Member

Choose a reason for hiding this comment

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

I have not! Sounds like a good addition to our Django project template. Opened datamade/how-to#115.

An impeccable style guide, however, does not prevent human error. Every coder has at least one horror story about an hours-long hunt for a missing semicolon that caused the (sometimes silent) crash of an entire program. At DataMade, we recommend [ESLint](https://eslint.org/) as a way to enforce a coherent JS syntax and automate the hunt for typos.

## jshint
We integrate `ESLint` into our workflow as a step in continuous integration—typically, when any commit is pushed to a feature branch. This means that we catch syntax errors early, but are not stymied during local development as we introduce new code and troubleshoot. For an example of what this looks like, see the Gatsby app cookiecutter template. Notable files:
Copy link
Member

Choose a reason for hiding this comment

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

How about a link to the template here?

An impeccable style guide, however, does not prevent human error. Every coder has at least one horror story about an hours-long hunt for a missing semicolon that caused the (sometimes silent) crash of an entire program.
- `package.json`: This setup requires installation of the package `eslint-config-react-app`. This is also where you can define the `yarn test` command used in `test.yml`
- `.eslintrc.js`: This file defines the syntax rules ESLint will follow. It's based on the `react-app` defaults, with minor modifications.
- `.github/workflows/test.yml`: This Âworkflow instructs Github to run the ESLint tests on every push.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- `.github/workflows/test.yml`: This Âworkflow instructs Github to run the ESLint tests on every push.
- `.github/workflows/test.yml`: This workflow instructs Github to run the ESLint tests on every push.


An impeccable style guide, however, does not prevent human error. Every coder has at least one horror story about an hours-long hunt for a missing semicolon that caused the (sometimes silent) crash of an entire program.
- `package.json`: This setup requires installation of the package `eslint-config-react-app`. This is also where you can define the `yarn test` command used in `test.yml`
- `.eslintrc.js`: This file defines the syntax rules ESLint will follow. It's based on the `react-app` defaults, with minor modifications.
Copy link
Member

Choose a reason for hiding this comment

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

What modifications?

Copy link
Member

Choose a reason for hiding this comment

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

If this differs from our style guide here https://github.com/datamade/node-style-guide, we ought to deprecate it, as well. (Never really got team buy in, so I think we can either delete or archive our fork.)

@hancush
Copy link
Member

hancush commented Sep 25, 2020

P.s., this looks awesome, @beamalsky! Thank you for helping to keep this documentation inline with our R&D. ❤️ Let me know if there's anything I can do to pitch in.

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.

2 participants