-
Notifications
You must be signed in to change notification settings - Fork 8
Nikko Bitmap Transformer Submission #24
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?
Nikko Bitmap Transformer Submission #24
Conversation
set up basic structure and files
created our constructor for the bitmap object we'll be creating
getting file data and making constructor
Yana conversions
…-fix added a single test and master fix
trying to fix odd git issue
Trying to fix mystery bug.
fix the branch
added cli interface
…s are now on the bmp-constructor file, color transformations can be called depending on which one is wanted
Yana stretch2
…ing, feels really close
Nikko final tests
cleaned unnecessary logs
Tests all pass!
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.
@npisciotti1 @radenska @sneakyneeks2 @darms Excellent job on this assignment! I've left a bunch of comments, some more nitpicky than others, but overall - nailed it!
"impliedStrict": true | ||
}, | ||
"extends": "eslint:recommended" | ||
} |
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.
@npisciotti1 @radenska @sneakyneeks2 @darms .eslintrc
looks good
# .nfs files are created when an open file is removed but is still being accessed | ||
.nfs* | ||
|
||
# End of https://www.gitignore.io/api/node,vim,osx,macos,linux |
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.
@npisciotti1 @radenska @sneakyneeks2 @darms .gitignore
looks good
|
||
|Darcy McCabe|Nikko Pisciotti|Yana Radenska | ||
|:---:|:---:|:---:| | ||
[GitHub Repos](https://github.com/darms) |[GitHub Repos](https://github.com/npisciotti1)| [GitHub Repos](https://github.com/radenska)| |
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.
@npisciotti1 @radenska @sneakyneeks2 @darms README
and associated documentation looks good
gulp.watch(['**/*.js', '!node_modules/**'], ['lint', 'test']); | ||
}); | ||
|
||
gulp.task('default', ['dev']); |
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.
@npisciotti1 @radenska @sneakyneeks2 @darms gulpfile.js
looks good
console.log('Format: node index <filename> <transformation type> Example: node index color-palette.bmp invert'); | ||
throw('error'); | ||
} | ||
fileReadHelper.getBitMap(makeBitMap); |
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.
@npisciotti1 @radenska @sneakyneeks2 @darms great job checking command line arguments and handling any errors if the proper filename/transformation type were not provided
additionally, great job creating and including a readFile
helper module for reusability
side note: no need to use ${__dirname}
in your require statements - simple relative paths are fine for this and have become an industry standard
} | ||
}; | ||
|
||
function selectTransformation() { |
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.
@npisciotti1 @radenska @sneakyneeks2 @darms great transform selector - this could be refactored to be a bit more DRY but i love the addition of it!
"keywords": [], | ||
"author": "", | ||
"license": "ISC" | ||
} |
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.
@npisciotti1 @radenska @sneakyneeks2 @darms package.json
looks good
}); | ||
}); | ||
}); | ||
}); |
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.
@npisciotti1 @radenska @sneakyneeks2 @darms indentation issues ;)
nice job checking the general data types that are associated with your bitmap constructor
}); | ||
}); | ||
}); | ||
}); |
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.
@npisciotti1 @radenska @sneakyneeks2 @darms this includes a decent test - as stated above, a better approach would be to encapsulate your readFile
calls into a read-file-helper
that can be exported and used on demand, including in your tests
done(); | ||
}); | ||
}); | ||
}); |
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.
@npisciotti1 @radenska @sneakyneeks2 @darms color constructor tests are awesome!!!! lots of em!!!
Submission questions/reflection on Canvas.