Refactor the Code to use (JavaScript The) "Good Parts" Linter

nelsonic commented 7 years ago

at present the code in this module works as expected. but... it does not follow our "style guide": https://github.com/dwyl/goodparts ideally, all our projects should follow this guide to ensure we have consistency across all our JS.

If anyone has the time/patience to re-factor the code in /lib to conform to goodparts please comment on this issue to show your interest.

`Note`: "Refactoring" is when you don't change any existing tests. `only update` the code in `/lib` files to use a different code style.

`Note2`: If you haven't already done so, please read and :star: the "Contributing Guide": https://github.com/dwyl/contributing to ensure that our "steps" are followed and someone can read your Pull Request without any delays. 👍

The task(s) are:

[x] clone the repo to your local machine
[x] create a new branch called goodparts-refactor
[x] npm install goodparts --savedev
[x] add the following line to the scripts section of package.json
```
"lint": "node_modules/.bin/goodparts lib/"
```
[x] run the linter npm run lint
[x] update this issue with a screenshot of the error messages you see in your console/terminal, so that we know how many there were ...
[x] now attempt to run the "fix" with the following command:
```
node_modules/.bin/goodparts lib/ --fix
```
That should "fix" a few of the "basic" code style issues, but it will not address any "major" ones.
[x] confirm the "fix" worked as expected by re-running the tests: npm test
[x] Now the "fun" part: re-thinking the code to follow an alternative (more beginner-friendly) code style...! This will involve quite a few changes dependent on how many things the goodparts (ESLint) is reporting...
- [x] each time you make a change re-run the tests and lint. My preferred way of doing this is to re-run my test & lint commands in two separate terminal windows/tabs using nodemon each time the file is updated.
[x] Once all the warnings raised by goodparts have been resolved and the code conforms to the styleguide, commit your changes referencing this issue in your commit message.
[x] finally, if that all worked, add the "Repo Badge" to the README.md to signal to everyone that this repo conforms to our code style guide.
[x] once the badge is added to the readme make a commit to that effect: git commit -m 'adds goodparts repo badge see: https://github.com/dwyl/repo-badges#goodparts-javascript-code-style-'
[x] push up your branch and create a pull request referencing this issue.

newswim commented 7 years ago

I should have some time in the next few days to do this, if someone hasn't gotten to it already.

newswim commented 7 years ago

After running `--fix`

newswim commented 7 years ago

What's the recommended version of node for this project? It may be helpful to add that into the package.json.

I've been getting errors when running npm run test for example, but these don't occur if I just run the commands directly from the shell. Wondering if it might be an npm/node version issue?

Again, these work just fine without routing them through npm-scripts.

`npm run test`

➜  sendemail git:(goodparts-refactor) ✗ npm run test

> sendemail@3.0.0 test /Users/dm/Documents/dev/dwyl/sendemail
> istanbul cover ./node_modules/tape/bin/tape ./test/*.js

=============================================================================
Writing coverage object [/Users/dm/Documents/dev/dwyl/sendemail/coverage/coverage.json]
Writing coverage reports at [/Users/dm/Documents/dev/dwyl/sendemail/coverage]
=============================================================================

=============================== Coverage summary ===============================
Statements   : 43.33% ( 13/30 )
Branches     : 10% ( 1/10 )
Functions    : 25% ( 1/4 )
Lines        : 44.83% ( 13/29 )
================================================================================

/Users/dm/Documents/dev/dwyl/sendemail/lib/index.js:9
__cov_0bPW1yJMw2v7xpvKYS1dnw.s['1']++;var AWS=require('aws-sdk');__cov_0bPW1yJMw2v7xpvKYS1dnw.s['2']++;AWS.config.region=process.env.AWS_REGION;__cov_0bPW1yJMw2v7xpvKYS1dnw.s['3']++;var ses=new AWS.SES();__cov_0bPW1yJMw2v7xpvKYS1dnw.s['4']++;var path=require('path');__cov_0bPW1yJMw2v7xpvKYS1dnw.s['5']++;var fs=require('fs');__cov_0bPW1yJMw2v7xpvKYS1dnw.s['6']++;var Handlebars=require('handlebars');__cov_0bPW1yJMw2v7xpvKYS1dnw.s['7']++;var COMPILED_TEMPLATES={};function set_template_directory(dir){__cov_0bPW1yJMw2v7xpvKYS1dnw.f['1']++;__cov_0bPW1yJMw2v7xpvKYS1dnw.s['9']++;if(!dir){__cov_0bPW1yJMw2v7xpvKYS1dnw.b['1'][0]++;__cov_0bPW1yJMw2v7xpvKYS1dnw.s['10']++;throw'Please Set a Template Directory';}else{__cov_0bPW1yJMw2v7xpvKYS1dnw.b['1'][1]++;}__cov_0bPW1yJMw2v7xpvKYS1dnw.s['11']++;var files=fs.readdirSync(dir);__cov_0bPW1yJMw2v7xpvKYS1dnw.s['12']++;process.env.TEMPLATE_DIRECTORY=dir;}__cov_0bPW1yJMw2v7xpvKYS1dnw.s['13']++;set_template_directory(process.
Please Set a Template Directory

npm ERR! Darwin 16.3.0
npm ERR! argv "/Users/dm/.nvm/versions/node/v7.4.0/bin/node" "/Users/dm/.nvm/versions/node/v7.4.0/bin/npm" "run" "test"
npm ERR! node v7.4.0
npm ERR! npm  v4.0.5
npm ERR! code ELIFECYCLE
npm ERR! sendemail@3.0.0 test: `istanbul cover ./node_modules/tape/bin/tape ./test/*.js`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the sendemail@3.0.0 test script 'istanbul cover ./node_modules/tape/bin/tape ./test/*.js'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the sendemail package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     istanbul cover ./node_modules/tape/bin/tape ./test/*.js
npm ERR! You can get information on how to open an issue for this project with:
npm ERR!     npm bugs sendemail
npm ERR! Or if that isn't available, you can get their info via:
npm ERR!     npm owner ls sendemail
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR!     /Users/dm/Documents/dev/dwyl/sendemail/npm-debug.log

`npm run lint`

npm ERR! Darwin 16.3.0
npm ERR! argv "/Users/dm/.nvm/versions/node/v7.4.0/bin/node" "/Users/dm/.nvm/versions/node/v7.4.0/bin/npm" "run" "lint"
npm ERR! node v7.4.0
npm ERR! npm  v4.0.5
npm ERR! code ELIFECYCLE
npm ERR! sendemail@3.0.0 lint: `goodparts lib/`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the sendemail@3.0.0 lint script 'goodparts lib/'.

nelsonic commented 7 years ago

@newswim looks like the lint --fix was a "too aggressive"... thanks for investigating this! 👍

newswim commented 7 years ago

Funny enough, it works when I re-installed node_modules using Yarn. I know we're avoiding, but this seems to be specific to npm version.

(this is the main reason i've started using Yarn, it's weird ability to solve inexplicables)

nelsonic commented 7 years ago

@newswim which module is causing issues?

newswim commented 7 years ago

Failed at the sendemail@3.0.0 lint script 'goodparts lib/'.

that's part of the output from npm run lint

newswim commented 7 years ago

more not-really-helpful npm output:

$ npm run lint
npm ERR! Darwin 16.3.0
npm ERR! argv "/Users/dm/.nvm/versions/node/v7.4.0/bin/node" "/Users/dm/.nvm/versions/node/v7.4.0/bin/npm" "run" "test"
npm ERR! node v7.4.0
npm ERR! npm  v4.0.5
npm ERR! code ELIFECYCLE
npm ERR! sendemail@3.0.0 test: `istanbul cover ./node_modules/tape/bin/tape ./test/*.js`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the sendemail@3.0.0 test script 'istanbul cover ./node_modules/tape/bin/tape ./test/*.js'.

nelsonic commented 7 years ago

Ok, that won't be an NPM issue though ... try running the tests without istanbul cover

newswim commented 7 years ago

Hope you like console dumps...

➜  sendemail git:(goodparts-refactor) ✗ npm run quick

> sendemail@3.0.0 quick /Users/dm/Documents/dev/dwyl/sendemail
> ./node_modules/tape/bin/tape ./test/*.js

/Users/dm/Documents/dev/dwyl/sendemail/lib/index.js:16
    throw 'Please Set a Template Directory';
    ^
Please Set a Template Directory

npm ERR! Darwin 16.3.0
npm ERR! argv "/Users/dm/.nvm/versions/node/v7.4.0/bin/node" "/Users/dm/.nvm/versions/node/v7.4.0/bin/npm" "run" "quick"
npm ERR! node v7.4.0
npm ERR! npm  v4.0.5
npm ERR! code ELIFECYCLE
npm ERR! sendemail@3.0.0 quick: `./node_modules/tape/bin/tape ./test/*.js`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the sendemail@3.0.0 quick script './node_modules/tape/bin/tape ./test/*.js'.

newswim commented 7 years ago

Do i need to throw that Template Directory var into .env ?

newswim commented 7 years ago

well, I have it working with Yarn for now and am using nodemon for 💻 'ing ---- gonna start the cleanup!

newswim commented 7 years ago

There are some issues with the tests, but I think that's out of the scope of this PR. Happy to investigate further, but I'm going to sleep now! 🌃

nelsonic commented 7 years ago

Great Work @newswim! sleep well! 😴

nelsonic commented 7 years ago

done 🎉

dwyl / sendemail