Skip to content

04-Bitmap- Zachary-Sugey-Jeremiah#14

Open
jtwalters25 wants to merge 36 commits intocodefellows-javascript-401d13:masterfrom
jtwalters25:master
Open

04-Bitmap- Zachary-Sugey-Jeremiah#14
jtwalters25 wants to merge 36 commits intocodefellows-javascript-401d13:masterfrom
jtwalters25:master

Conversation

@jtwalters25
Copy link

Bitmap Transformation completed after many challenges.

zcrumbo and others added 30 commits February 16, 2017 15:39
Created transform function that doesn't do what we think it should
got the transform function working
refactored transform function - split functions into export files
added command line argument support
Test complete on for components of the bitmap constructor function
added documentation and extra example pics
fixes and proof reading of doc
Copy link
Contributor

@bnates bnates left a comment

Choose a reason for hiding this comment

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

@jtwalters25 @zcrumbo @sueanyv nice job! i've left some general nitpicky comments in your PR but, overall, y'all did great and it's clear you put in a bunch of time and effort to get this to work.

"impliedStrict": true
},
"extends": "eslint:recommended"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@jtwalters25 @zcrumbo @sueanyv .eslintrc looks good

# Windows shortcuts
*.lnk

# End of https://www.gitignore.io/api/node,osx,windows,linux No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

@jtwalters25 @zcrumbo @sueanyv .gitignore looks good

node index.js invert random bluescale grayscale
~~~~
<br><br>
[Back to the top](#top)
Copy link
Contributor

Choose a reason for hiding this comment

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

@jtwalters25 @zcrumbo @sueanyv generally speaking, this README and associated documentation looks good - for future projects, i'd like to see more of an emphasis on using modern markdown constructs - i'll work with you on this more as we approach mid-quarter projects

gulp.watch(['**/*.js', '!node_modules/**'], ['lint', 'test']);
});

gulp.task('default', ['dev']);
Copy link
Contributor

Choose a reason for hiding this comment

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

@jtwalters25 @zcrumbo @sueanyv gulpfile.js looks good

}
return
}
throw new Error('You must supply a transform command: grayscale, invert, bluescale or random)'); No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

@jtwalters25 @zcrumbo @sueanyv index file looks good - great job including and implementing command line arguments

});
};

exports.write = function(err, data, fileName){
Copy link
Contributor

Choose a reason for hiding this comment

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

@jtwalters25 @zcrumbo @sueanyv great job encapsulating your writeFile call for reusability

readWrite.write(null, bitmap, outputFile);
}
});
}; No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

@jtwalters25 @zcrumbo @sueanyv transform methods look good - that said, a better approach would be to make these prototype methods of a transform constructor - this is simply a cleaner pattern and makes these methods easier to test and chain

"gulp-mocha": "^3.0.1",
"mocha": "^3.2.0"
}
} No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

@jtwalters25 @zcrumbo @sueanyv package.json looks good

});

});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

@jtwalters25 @zcrumbo @sueanyv tests look good - nice job checking metadata about the bitmap, including the size and type

// }
// });
// });
// });
Copy link
Contributor

Choose a reason for hiding this comment

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

@jtwalters25 @zcrumbo @sueanyv large comment blocks should be removed from the master branch - these should be contained on dev branches

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.

4 participants