-
Notifications
You must be signed in to change notification settings - Fork 14
Day 1 & 2 assignment submission #15
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| { | ||
| "rules": { | ||
| "comma-dangle": ["error", "always-multiline"], | ||
| "no-console": "off", | ||
| "indent": [ "error", 2 ], | ||
| "quotes": [ "error", "single" ], | ||
| "semi": ["error", "always"], | ||
| "linebreak-style": [ "error", "unix" ] | ||
| }, | ||
| "env": { | ||
| "es6": true, | ||
| "node": true, | ||
| "mocha": true, | ||
| "jasmine": true | ||
| }, | ||
| "globals": { | ||
| }, | ||
| "ecmaFeatures": { | ||
| "modules": true, | ||
| "experimentalObjectRestSpread": true, | ||
| "impliedStrict": true | ||
| }, | ||
| "extends": "eslint:recommended" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| # Created by https://www.gitignore.io/api/node,osx,linux | ||
|
|
||
| ### Node ### | ||
| # Logs | ||
| logs | ||
| *.log | ||
| npm-debug.log* | ||
|
|
||
| # Runtime data | ||
| pids | ||
| *.pid | ||
| *.seed | ||
| *.pid.lock | ||
|
|
||
| # Directory for instrumented libs generated by jscoverage/JSCover | ||
| lib-cov | ||
|
|
||
| # Coverage directory used by tools like istanbul | ||
| coverage | ||
|
|
||
| # nyc test coverage | ||
| .nyc_output | ||
|
|
||
| # Grunt intermediate storage (http://gruntjs.com/creating-plugins#storing-task-files) | ||
| .grunt | ||
|
|
||
| # node-waf configuration | ||
| .lock-wscript | ||
|
|
||
| # Compiled binary addons (http://nodejs.org/api/addons.html) | ||
| build/Release | ||
|
|
||
| # Dependency directories | ||
| node_modules | ||
| jspm_packages | ||
|
|
||
| # Optional npm cache directory | ||
| .npm | ||
|
|
||
| # Optional eslint cache | ||
| .eslintcache | ||
|
|
||
| # Optional REPL history | ||
| .node_repl_history | ||
|
|
||
|
|
||
| ### OSX ### | ||
| *.DS_Store | ||
| .AppleDouble | ||
| .LSOverride | ||
|
|
||
| # Icon must end with two \r | ||
| Icon | ||
|
|
||
|
|
||
| # Thumbnails | ||
| ._* | ||
|
|
||
| # Files that might appear in the root of a volume | ||
| .DocumentRevisions-V100 | ||
| .fseventsd | ||
| .Spotlight-V100 | ||
| .TemporaryItems | ||
| .Trashes | ||
| .VolumeIcon.icns | ||
| .com.apple.timemachine.donotpresent | ||
|
|
||
| # Directories potentially created on remote AFP share | ||
| .AppleDB | ||
| .AppleDesktop | ||
| Network Trash Folder | ||
| Temporary Items | ||
| .apdisk | ||
|
|
||
|
|
||
| ### Linux ### | ||
| *~ | ||
|
|
||
| # temporary files which can be created if a process still has a handle open of a deleted file | ||
| .fuse_hidden* | ||
|
|
||
| # KDE directory preferences | ||
| .directory | ||
|
|
||
| # Linux trash folder which might appear on any partition or disk | ||
| .Trash-* |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| 'use strict'; | ||
|
|
||
| var greetings = require('./lib/greet.js'); | ||
| var name = process.argv[2]; | ||
|
|
||
| console.log(greetings.greet(name)); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| 'use strict'; | ||
|
|
||
| const gulp = require('gulp'); | ||
| const eslint = require('gulp-eslint'); | ||
| const mocha = require('gulp-mocha'); | ||
|
|
||
| gulp.task('test', function() { | ||
| gulp.src('./test/*-test.js', {read: false}) | ||
| .pipe(mocha({reporter: 'nyan'})); | ||
| }); | ||
|
|
||
| gulp.task('lint', function(){ | ||
| return gulp.src(['**/*.js','!node_modules/**']) | ||
| .pipe(eslint()) | ||
| .pipe(eslint.format()) | ||
| .pipe(eslint.failAfterError()); | ||
| }); | ||
|
|
||
| gulp.task('dev', function() { | ||
| gulp.watch(['**/*.js', '!node_modules/**'],['test', 'lint']); | ||
| }); | ||
|
|
||
| gulp.task('default', function() { | ||
| console.log('Now tracking all updates & additions to your node modules...'); | ||
| gulp.watch(['node_modules/**'],[function(){ | ||
| console.log('You added or changed your modules!'); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that you added console.logs to make your watch (initially running and triggering the tasks therein) more noticeable! |
||
| }]); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| 'use strict'; | ||
| exports = module.exports = {}; | ||
|
|
||
| exports.greet = function(name){ | ||
| if (arguments.length === 0 || arguments[0] === undefined) throw new Error('missing a name'); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice work using the arguments object! This is simply a stylistic preference, but you could alternatively write
which does the same thing. If arguments.length is equal to 0, arguments[0] would also be undefined, because it would not exist, so your OR operator is unnecessary. That would also make arguments.length falsey (because 0 is a falsey value) and !false is the same as true, meaning the code within the if block will be executed if there is no argument provided. This is a more standard practice. |
||
| return 'hello ' + name ; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're emphasizing string interpolation for this class, so give it a shot! You should start using this in all of your labs in place of string concatenation. You could refactor this line like so: |
||
| }; | ||
|
|
||
| exports.sayGoodbye = function(){ | ||
| return('Seeya'); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| { | ||
| "name": "day1", | ||
| "version": "1.0.0", | ||
| "description": "", | ||
| "main": "index.js", | ||
| "directories": { | ||
| "test": "test" | ||
| }, | ||
| "dependencies": {}, | ||
| "devDependencies": { | ||
| "fs": "0.0.1-security", | ||
| "gulp": "3.9.1", | ||
| "gulp-changed": "1.3.2", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Were you experimenting with fs and gulp-changed? I'm not familiar with gulp-changed! :) Best practice is to remove libraries that you don't end up using from your package.json. |
||
| "gulp-eslint": "3.0.1", | ||
| "gulp-mocha": "3.0.1", | ||
| "mocha": "3.0.2" | ||
| }, | ||
| "scripts": { | ||
| "test": "mocha" | ||
| }, | ||
| "author": "", | ||
| "license": "ISC" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| 'use strict'; | ||
| const greetings = require ('../lib/greet'); | ||
| const assert = require('assert'); | ||
|
|
||
| describe('testing module greet', function(){ | ||
| describe('testing #sayHello()', function(){ | ||
| it('should return "name" when passed "name"', function(){ | ||
| let result = greetings.greet('name'); | ||
| assert.ok(result === 'hello name', 'was not hello name'); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As per Duncan's comment in class today, when you're evaluating equality, it's a best practice to use assert.equals instead of assert.ok (although they both work :)). |
||
| }); | ||
|
|
||
| it('should throw a missing a name error if given no argument', function(){ | ||
| assert.throws(function() { | ||
| greetings.greet(); | ||
| }, 'shoulda thrown that err'); | ||
| }); | ||
|
|
||
| it('should process an input from CLI', function() { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test should be extracted into another test-file and test the app.js file. See Duncan's in-class code for elaboration :) Nice work thinking about mocking up process.argv[2]! |
||
| var name = process.argv[2] = 'adam'; | ||
| let result = greetings.greet(name); | ||
| assert.ok(result === 'hello ' + name, 'did not take CLI input'); | ||
| }); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job trying the bonus objective! You can't test something that is console.logged, so it would be better to construct a function that returns a call greetings.greet, pass in name (i.e. process.argv[2]) to greetings.greet, and call that outer function further down so that it runs in the command line. You can see an example of this in Duncan's class code. You would then test the app.js file as a separate module and set process.argv[2] explicitly in that test file.
You could console.log the result of the call before returning it if you want to be able to see it in the CLI, but you should still return it so that it is testable.