Skip to content

Brian, Cayla, Remi #22

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

Open
wants to merge 29 commits into
base: master
Choose a base branch
from

Conversation

brials
Copy link

@brials brials commented Feb 20, 2017

No description provided.

brials and others added 29 commits February 16, 2017 13:53
completed constructor and write
added tests and refactored code to include callbacks for testing purp…
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.

Excellent job on this assignment. Your team should be proud of what you built - it's fairly clean and your ability to modularize the bitmap file reader for testing purposes and inclusion in other modules was great. Keep it up!

"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.

@brials @remilonwheels @caylazabel .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
Copy link
Contributor

Choose a reason for hiding this comment

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

@brials @remilonwheels @caylazabel .gitignore looks good!


More transformations are to come!
<br>
ⒸCodeFellows 2017 Team Bad Hair Day
Copy link
Contributor

Choose a reason for hiding this comment

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

@brials @remilonwheels @caylazabel README and associated markdown look good

return fileColorTransform(files, colorTransforms);


function fileColorTransform(files, colorTransforms) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@brials @remilonwheels @caylazabel might be best to pull this color transformer function out of your transformBMP function for reusability cleanliness and readability

transformBMP(filesToConvert);

function transformBMP(files) {
let colorTransforms = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

@brials @remilonwheels @caylazabel good use of an empty array pattern for filling the array in the subsequent for loop

return [ grayLevel, grayLevel, grayLevel, color[3] ];
};

Bitmap.colorTransforms = [ 'invertTransform', 'blueTransform', 'grayTransform' ];
Copy link
Contributor

Choose a reason for hiding this comment

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

@brials @remilonwheels @caylazabel transform static methods and related logic look good!

"gulp-mocha": "^3.0.1",
"mocha": "^3.2.0"
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@brials @remilonwheels @caylazabel package.json looks as expected and devDependencies houses the appropriate data

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

Choose a reason for hiding this comment

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

@brials @remilonwheels @caylazabel tests are a bit light but overall test the primary expected results from an instantiated bitmap

describe('File Reader Module', function(){
describe('with an improper file path',function(){
it('should return an error', function(done) {
fileHelper('img/badPath.bmp', 'palette', 'invertTransform', function(err){
Copy link
Contributor

Choose a reason for hiding this comment

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

@brials @remilonwheels @caylazabel as stated in your bitmap-file-helper, excellent pattern of wrapping your file reader so that you can test - great job writing this particular test to check for errors

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

Choose a reason for hiding this comment

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

@brials @remilonwheels @caylazabel returned bitmap data tests are generally excellent albeit a bit light

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