Skip to content

Gary test template #19

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 13 commits into
base: master
Choose a base branch
from

Conversation

GLSea1979
Copy link

No description provided.

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.

@GLSea1979 @0blu3 @noahgribbin - Overall, good job on this assignment. There's a real need for modularity that isn't present at this current iteration but we'll keep talking about it and the creation/exposure of these modules will become second nature soon

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

@GLSea1979 @0blu3 @noahgribbin .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.

@GLSea1979 @0blu3 @noahgribbin .gitignore looks good

module.exports = exports = {};

exports.readContent = function(callback) {
fs.readFile(`${__dirname}/../img/palette-bitmap.bmp`, function(err, data) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@GLSea1979 @0blu3 @noahgribbin the function that you use to read a bitmap and subsequently add data as part of the constructor should be extracted out of this readContent method - it's best to create a helper file (ex: bitmap-file-reader) that encapsulates your readFile call and allows you to dynamically pass in a filename for reuse in other modules and for proper testing

this.imageStart = bitmap.readInt32LE(10);
this.pixelArray = bitmap.toString('hex', this.imageStart, this.size);


Copy link
Contributor

Choose a reason for hiding this comment

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

@GLSea1979 @0blu3 @noahgribbin nitpicky time - got some unnecessary whitespace ;)


};
var test = new Image;
console.log('var test:', test);
Copy link
Contributor

Choose a reason for hiding this comment

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

@GLSea1979 @0blu3 @noahgribbin for future reference, console.log statements should only exist on branches outside of master - master should only contain production code

var avg = (data[i] + data[i + 1] + data[i + 2]) / 3;
data[i] = avg;
data[i + 1] = avg;
data[i + 2] = avg;
Copy link
Contributor

Choose a reason for hiding this comment

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

@GLSea1979 @0blu3 @noahgribbin grayscale manipulations look good


exports.readContent.prototype.white = function() {
this.pixelArray.fill(255);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

@GLSea1979 @0blu3 @noahgribbin invert and white transforms look good


g.readContent(function (err, data) {

});
Copy link
Contributor

Choose a reason for hiding this comment

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

@GLSea1979 @0blu3 @noahgribbin any reason this file exists?

"chai": "^3.5.0",
"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.

@GLSea1979 @0blu3 @noahgribbin package.json looks as expected

// read the buffer and compare to our object
})
})
});
Copy link
Contributor

Choose a reason for hiding this comment

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

@GLSea1979 @0blu3 @noahgribbin this needs some actual tests - no worries for now, the overall structure of your describe and it blocks look good - you'll get more comfortable with the actual assertion tests as we continue on in this class

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