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

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

Merged
merged 2 commits into from
Dec 15, 2016

Conversation

malept
Copy link
Contributor

@malept malept commented Dec 11, 2016

This is a continuation of #252. I would have used the existing commits in that PR, but the branch was no longer available.

Since I added fs-extra for the ensureDir function, I also replaced rimraf usage with the equivalent fs-extra functionality.

Fixes #240, #339.
Closes #252.

TODO

  • Use a release of cachedir that has Windows support

@malept
Copy link
Contributor Author

malept commented Dec 11, 2016

Sigh. I forgot that env-paths doesn't work with Node < 4.

@LinusU
Copy link
Contributor

LinusU commented Dec 12, 2016

I have merged your PR to node-cachedir, you can depend on ^1.1.0 now :)

@LinusU
Copy link
Contributor

LinusU commented Dec 12, 2016

Nice 👌

@LinusU
Copy link
Contributor

LinusU commented Dec 12, 2016

Any idea about the test failure?

@malept
Copy link
Contributor Author

malept commented Dec 12, 2016

Not particularly. It runs fine locally (otherwise I wouldn't be able to commit). I think it has something to do with how the CI is set up, because it looks like AppVeyor has one error and Travis has another.

@jimthedev
Copy link
Member

I'll look into CI.

@jimthedev
Copy link
Member

Still looking into this but this is what I get when I run npm install && npm test locally on your fork.


> commitizen@0.0.0-semantically-released test /Users/jim.cummins/projects/cz-forks/cz-cli
> nyc --require babel-core/register _mocha -- test/tests/index.js



  adapter
    1) resolves adapter paths
    2) gets adapter prompter functions

  git-cz
    bootstrap
      when config is provided
        ✓ passes config to useGitCzStrategy
      when config is not provided
        and the config is returned from configLoader.load
          ✓ uses config from configLoader.load()
        and the config is not returned from configLoader.load
          ✓ tells commitizen to use the git strategy

  commit
    3) should commit simple messages
    4) should commit message with quotes
    5) should commit multiline messages
    6) should allow to override git commit options

  configLoader
    ✓ errors appropriately for invalid json
    ✓ parses json files with comments
    ✓ normalizes package.json configs
    ✓ normalizes .cz.json configs
    ✓ normalizes .czrc configs

  init
    7) installs an adapter with --save-dev
    8) installs an adapter with --save
    9) errors on previously installed adapter
    10) succeeds if force is true
    11) installs an adapter without --save-exact
    12) installs an adapter with --save-exact

  staging
    ✓ should determine if a repo is clean (5853ms)

  common util
    ✓ isArray determines if array is passed
    ✓ isFunction determines if a function is passed
    ✓ isString determines if string is passed


  12 passing (6m)
  12 failing

  1) adapter resolves adapter paths:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)


  2) adapter gets adapter prompter functions:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)


  3) commit should commit simple messages:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)


  4) commit should commit message with quotes:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)


  5) commit should commit multiline messages:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)


  6) commit should allow to override git commit options:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)


  7) init installs an adapter with --save-dev:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)


  8) init installs an adapter with --save:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)


  9) init errors on previously installed adapter:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)


  10) init succeeds if force is true:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)


  11) init installs an adapter without --save-exact:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)


  12) init installs an adapter with --save-exact:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)

@malept
Copy link
Contributor Author

malept commented Dec 12, 2016

Strange. I just checked out my branch on a machine that has never had commitizen installed on it, ran npm install && npm test, and all tests pass.

@jimthedev
Copy link
Member

@malept I wonder if it has to do with the fact that I have a commitizen adapter installed globally so that I can use the cli in directories that don't have an adapter initialized.

@malept
Copy link
Contributor Author

malept commented Dec 12, 2016

That would explain the error you're getting.

@jimthedev
Copy link
Member

Yeah renaming my ~/.czrc file made it so that all tests pass locally. Working on CI still.

@jimthedev
Copy link
Member

I have a branch where I am working on CI errors. We were not reporting git log errors in test.

@jimthedev
Copy link
Member

Here are the actual errors we're getting.
https://travis-ci.org/commitizen/cz-cli/jobs/183566039

Looking at our builds, this was introduced earlier so it is not specifically due to this PR. Could be a git config change on Travis that we didn't catch. Either way you can track this on #401.

@jimthedev
Copy link
Member

@malept I just pushed a fix for CI to master so if you merge master into your branch you should get those CI changes and your tests should pass.

@malept
Copy link
Contributor Author

malept commented Dec 14, 2016

@jimthedev thanks for the heads up, I will rebase/squash later tonight.

@jimthedev
Copy link
Member

Looks good. Our appveyor tests are failing just due to how long they run but travis seems happy.

@jimthedev jimthedev merged commit 27fb1ae into commitizen:master Dec 15, 2016
@jimthedev
Copy link
Member

Closes #339 as well.

@malept malept deleted the use-cache-dir branch December 15, 2016 15:08
jimthedev added a commit that referenced this pull request Dec 19, 2016
* master:
  docs(install): Add documentation for installing and running locally (#300)
  fix(tests): fix tests when a global config is present (#405)
  test(adapter): add test for scoped npm modules
  fix(adapter): add support for scoped adapters
  docs: Add cz-emoji to tools list (#404)
  feat(commit): use OS-specific cache dir for commitizen.json instead of home-or-tmp (#400)
  fix(npmignore): ignore more unnecessary files (#393)
  docs: add vscode-commitizen to tools list (#397)
  Ci issues with commits (#402)
  chore(ci): add node versions and say git version in ci
  docs: replace sudo mention with link to how to fix EACCES error
jimthedev added a commit that referenced this pull request Jan 2, 2017
* master:
  fix(package): "main" property within "package.json" (#409)
  style: Add linting using eslint (#406)
  ci(appveyor): remove appveyor on finish script
  ci(appveyor): disable collection of artifacts
  ci(tests): on windows run tests as node4/npm3
  ci(travis): update semantic-release to 6.3.5 and update travis config
  chore(package): update nyc to version 10.0.0 (#392)
  docs(install): Add documentation for installing and running locally (#300)
  fix(tests): fix tests when a global config is present (#405)
  test(adapter): add test for scoped npm modules
  fix(adapter): add support for scoped adapters
  docs: Add cz-emoji to tools list (#404)
  feat(commit): use OS-specific cache dir for commitizen.json instead of home-or-tmp (#400)
  fix(npmignore): ignore more unnecessary files (#393)
  docs: add vscode-commitizen to tools list (#397)
  Ci issues with commits (#402)
  chore(ci): add node versions and say git version in ci
  docs: replace sudo mention with link to how to fix EACCES error
jimthedev added a commit that referenced this pull request Jan 2, 2017
* master: (25 commits)
  chore(package): update lodash to version 4.17.2 (#389)
  chore(package): update ghooks to version 1.3.2 (#277)
  chore(package): update babel-preset-stage-2 to version 6.18.0 (#371)
  chore(package): update babel-preset-es2015 to version 6.18.0 (#370)
  chore(package): update babel-cli to version 6.18.0 (#369)
  chore(package): update axios to version 0.15.2 (#366)
  chore(package): update find-node-modules to version 1.0.4 (#346)
  fix(package): "main" property within "package.json" (#409)
  style: Add linting using eslint (#406)
  ci(appveyor): remove appveyor on finish script
  ci(appveyor): disable collection of artifacts
  ci(tests): on windows run tests as node4/npm3
  ci(travis): update semantic-release to 6.3.5 and update travis config
  chore(package): update nyc to version 10.0.0 (#392)
  docs(install): Add documentation for installing and running locally (#300)
  fix(tests): fix tests when a global config is present (#405)
  test(adapter): add test for scoped npm modules
  fix(adapter): add support for scoped adapters
  docs: Add cz-emoji to tools list (#404)
  feat(commit): use OS-specific cache dir for commitizen.json instead of home-or-tmp (#400)
  ...
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.

3 participants