Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

cache.js: preparation for git module (factor out getCacheStat) #4765

Closed
wants to merge 2 commits into from

Conversation

robertkowalski
Copy link
Contributor

This is one of the first steps to factor out the git part of
cache.js into a separate module which can be tested separately.

@robertkowalski
Copy link
Contributor Author

Note: @isaacs already got publish rights for the module, will start a transfer if we merge this in

, fs = require("fs")
, fixtures = path.join(__dirname, "fixtures")

var npm = { cache: path.join(fixtures, "mycache") }
Copy link
Contributor

Choose a reason for hiding this comment

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

If all you use from the npm object is the cache property, perhaps this module should just accept a cache value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are absolutely right. Missed the forest for the trees :)

This is one of the first steps to factor out the git part of
`cache.js` into a separate module which can be tested separately.
@robertkowalski
Copy link
Contributor Author

fixed! :)

@domenic
Copy link
Contributor

domenic commented Feb 24, 2014

So this LGreatTM but I want to make sure @isaacs is on board with this direction of refactoring cache.js. I know our original and/or long-term plan was #4159 but this might be a nice way to go in the meantime. Actually it might still be necessary anyway even after that.

@robertkowalski
Copy link
Contributor Author

Side-note: #4159 discusses mainly caching http agents, not how the git part should work. I'm planning to integrate the git module into npm-fetch anyway which is also suggested there.

@domenic
Copy link
Contributor

domenic commented May 11, 2014

Well, I guess @othiym23's recent cache refactor supersedes this...

@domenic domenic closed this May 11, 2014
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.

None yet

2 participants