Skip to content
This repository has been archived by the owner on Jul 3, 2019. It is now read-only.

feat(read): change hasContent to return {sri, size} #88

Merged
merged 4 commits into from
Apr 22, 2017
Merged

feat(read): change hasContent to return {sri, size} #88

merged 4 commits into from
Apr 22, 2017

Conversation

cilice
Copy link
Contributor

@cilice cilice commented Apr 22, 2017

Some changes were needed to handle the changes of pickContentSri result,
which is now {sri, lstat} instead of sri.

Fixes #87

Alexander Plavinski added 3 commits April 22, 2017 19:25
Some changes were needed to handle the changes of pickContentSri result,
which is now `{sri, lstat}` instead of `sri`.

Fixes #87
Copy link
Owner

@zkat zkat left a comment

Choose a reason for hiding this comment

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

So far so good! Thanks a bunch for this. I had a couple of nits, but also I'd like tests for the size functionality.

.catch({code: 'ENOENT'}, () => false)
.catch({code: 'EPERM'}, err => {
if (process.platform !== 'win32') {
throw err
} else {
return false
}
}).then(sri => sri || false)
}).then((content) => {
Copy link
Owner

Choose a reason for hiding this comment

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

No parens around single-var fat-arrows

@@ -34,7 +35,8 @@ function readStream (cache, integrity, opts) {
const stream = new PassThrough()
pickContentSri(
cache, integrity
).then(sri => {
).then((content) => {
Copy link
Owner

Choose a reason for hiding this comment

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

No parens around single-var fat-arrows

@@ -13,7 +13,8 @@ BB.promisifyAll(fs)
module.exports = read
function read (cache, integrity, opts) {
opts = opts || {}
return pickContentSri(cache, integrity).then(sri => {
return pickContentSri(cache, integrity).then((content) => {
Copy link
Owner

Choose a reason for hiding this comment

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

No parens around single-var fat-arrows

@@ -8,7 +8,8 @@ const rimraf = BB.promisify(require('rimraf'))

module.exports = rm
function rm (cache, integrity) {
return hasContent(cache, integrity).then(sri => {
return hasContent(cache, integrity).then((content) => {
Copy link
Owner

Choose a reason for hiding this comment

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

No parens around single-var fat-arrows

@@ -52,34 +54,33 @@ function readStream (cache, integrity, opts) {
module.exports.hasContent = hasContent
function hasContent (cache, integrity) {
Copy link
Owner

Choose a reason for hiding this comment

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

}).then(sri => sri || false)
}).then((content) => {
if (!content.sri) return false
return ({ sri: content.sri, size: content.stat.size || 0 })
Copy link
Owner

Choose a reason for hiding this comment

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

Probably don't need this || 0 anymore!

@cilice
Copy link
Contributor Author

cilice commented Apr 22, 2017

Changed the bits you adressed and tried to make a test that makes sense, if I can somehow make it better, just tell me :)

Copy link
Owner

@zkat zkat left a comment

Choose a reason for hiding this comment

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

Wonderful. Thank you!

@zkat zkat merged commit bad6c49 into zkat:latest Apr 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants