Skip to content
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

(commit): moved cache (#240) #252

Closed
wants to merge 3 commits into from
Closed

(commit): moved cache (#240) #252

wants to merge 3 commits into from

Conversation

dealloc
Copy link
Contributor

@dealloc dealloc commented Jun 9, 2016

Moved the cache from the home folder to a designated config folder

This fixes #240 by moving the commitizen.json cache file out of the home folder

Moved the cache from the home folder to a designated config folder
@@ -1,5 +1,5 @@
import path from 'path';
import homeOrTmp from 'home-or-tmp';
import ConfigDir from 'application-config-path';
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that the convention is that initial uppercase letter is for classes, can you change this name to configDir instead? thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certainly, hang on :)

Fixed the casing of ConfigDir to match guidelines
@dealloc
Copy link
Contributor Author

dealloc commented Jun 9, 2016

@LinusU I don't see why the builds should fail, could this be related to #205 ?

@LinusU
Copy link
Contributor

LinusU commented Jun 9, 2016

👍 I'll look into the failing build when I'm not at work

@dealloc
Copy link
Contributor Author

dealloc commented Jun 12, 2016

I'm pulling my hairs out over this one, I literally replaced a string with another string ?_?

@jimthedev
Copy link
Member

jimthedev commented Jun 13, 2016

@dealloc @LinusU I could be wrong, but I am pretty sure this is failing simply because if the directory doesn't exist, it isn't being created first. Your PR should probably create that directory before trying to create the file.

@dealloc
Copy link
Contributor Author

dealloc commented Jun 13, 2016

I'm going to give that a try

Create the cache directory if it doesn't exist yet.
@LinusU
Copy link
Contributor

LinusU commented Jun 13, 2016

@dealloc Maybe it's better to use this package since it's actually meant for cache? The current one is meant for config which isn't really what this is...

https://www.npmjs.com/package/cachedir

@dealloc
Copy link
Contributor Author

dealloc commented Jun 13, 2016

does this also handle creating the directories etc? or would I need to handle that myself?

EDIT: a quick skim gives me the impression this more or less does the same as the package you suggested before (generate OS specific strings) but it seems that this package will throw on Windows, which is probably not really what we want

@LinusU
Copy link
Contributor

LinusU commented Jun 14, 2016

This works the same, it will just give back the path.

Ahh, you're right, we should probably fix that first. PR welcome if you have the time, otherwise I'll try and fix it as soon as possible.

@dealloc
Copy link
Contributor Author

dealloc commented Jun 15, 2016

I suggest for now we could just use the package I already included and just update it when the other one is fixed. The main thing right now is that it won't commit and I have no clue why (I suspect #205 is the culprit here).

malept added a commit to malept/cz-cli that referenced this pull request Dec 11, 2016
malept added a commit to malept/cz-cli that referenced this pull request Dec 15, 2016
jimthedev pushed a commit that referenced this pull request Dec 15, 2016
…f home-or-tmp (#400)

* feat(commit): use OS-specific cache dir for commitizen.json instead of home-or-tmp

Based on #252.

Fixes #240, #252, #339

* refactor(tests): replace rimraf with fs-extra
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.

Configure cache
3 participants