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

Make .cz.json part of package.json #9

Closed
boennemann opened this issue Aug 23, 2015 · 18 comments
Closed

Make .cz.json part of package.json #9

boennemann opened this issue Aug 23, 2015 · 18 comments

Comments

@boennemann
Copy link

This project looks very promising, thank your for this!
What shies me away as a maintainer is that I have to add another dotfile to my repository.
As the .cz.json is mostly one line, and also json, it would be nice if commitizen could just look up a "cz" field the package.json.

What do you think?

(Sorry if I missed the a previous discussion on this, couldn't find one.)

@jimthedev
Copy link
Member

Hi there! You've hit on a really good point and I apologize for this not being in the docs Already, but you can actually already do this in your package.json under the cz key. The 3files that the config loader uses are (in order): package.json, .czrc, and .cz.json. These are defined here: https://github.com/commitizen/cz-cli/blob/master/src/configLoader.js

when I get back from vacation I'll cut a new docs release.

@boennemann
Copy link
Author

Amazing, thank you! Will give this a shot soon in my projects!

I'll leave this open, so it can be closed once the docs landed ;)

@jimthedev
Copy link
Member

Cool. Just one correction. The package.json key is czConfig.

@kentcdodds
Copy link
Member

I actually demonstrate this in my egghead lessons :-) because .cz.json is so small there's no reason for an entire file for it :-)

@accraze
Copy link
Contributor

accraze commented Oct 4, 2015

@jimthedev I wrote up some quick docs for this issue but am getting a 403 when I try to push it upstream... any chance you can add me to the repo so I can make a PR?

@kentcdodds
Copy link
Member

Did you Fork the repo? You should Fork it, then push it to your fork and
make a PR from that.

On Sun, Oct 4, 2015, 11:06 AM A. Craze notifications@github.com wrote:

@jimthedev https://github.com/jimthedev I wrote up some quick docs for
this issue but am getting a 403 when I try to push it upstream... any
chance you can add me to the repo so I can make a PR?


Reply to this email directly or view it on GitHub
#9 (comment).

@jimthedev
Copy link
Member

Thanks @accraze!!! Merged! Also thanks to @kentcdodds for merge and rebase which let me merge from my phone!

@kentcdodds
Copy link
Member

Can you use bookmarklets on your phone? How?

On Sun, Oct 4, 2015, 1:22 PM Jim Cummins notifications@github.com wrote:

Thanks @accraze https://github.com/accraze!!! Merged! Also thanks to
@kentcdodds https://github.com/kentcdodds for merge and rebase which
let me merge from my phone!


Reply to this email directly or view it on GitHub
#9 (comment).

@jimthedev
Copy link
Member

You have to be in Desktop mode. To get there you scroll to the very bottom
of the mobile view and click 'Desktop version'
On Sun, Oct 4, 2015 at 2:29 PM Kent C. Dodds notifications@github.com
wrote:

Can you use bookmarklets on your phone? How?

On Sun, Oct 4, 2015, 1:22 PM Jim Cummins notifications@github.com wrote:

Thanks @accraze https://github.com/accraze!!! Merged! Also thanks to
@kentcdodds https://github.com/kentcdodds for merge and rebase which
let me merge from my phone!


Reply to this email directly or view it on GitHub
#9 (comment).

Reply to this email directly or view it on GitHub
#9 (comment).

@kentcdodds
Copy link
Member

You sure you're not just talking about github's normal merge button, not my
bookmarklet? Or can you select the bookmarklet and it works on that page?

On Sun, Oct 4, 2015 at 1:33 PM Jim Cummins notifications@github.com wrote:

You have to be in Desktop mode. To get there you scroll to the very bottom
of the mobile view and click 'Desktop version'
On Sun, Oct 4, 2015 at 2:29 PM Kent C. Dodds notifications@github.com
wrote:

Can you use bookmarklets on your phone? How?

On Sun, Oct 4, 2015, 1:22 PM Jim Cummins notifications@github.com
wrote:

Thanks @accraze https://github.com/accraze!!! Merged! Also thanks to
@kentcdodds https://github.com/kentcdodds for merge and rebase which
let me merge from my phone!


Reply to this email directly or view it on GitHub
<#9 (comment)
.

Reply to this email directly or view it on GitHub
#9 (comment).


Reply to this email directly or view it on GitHub
#9 (comment).

@jimthedev
Copy link
Member

You can select the bookmarklet from your bookmarks while on the page. It
pops up a webtask window and rebases and merges. I'm in chrome on iOS
On Sun, Oct 4, 2015 at 2:48 PM Kent C. Dodds notifications@github.com
wrote:

You sure you're not just talking about github's normal merge button, not my
bookmarklet? Or can you select the bookmarklet and it works on that page?

On Sun, Oct 4, 2015 at 1:33 PM Jim Cummins notifications@github.com
wrote:

You have to be in Desktop mode. To get there you scroll to the very
bottom
of the mobile view and click 'Desktop version'
On Sun, Oct 4, 2015 at 2:29 PM Kent C. Dodds notifications@github.com
wrote:

Can you use bookmarklets on your phone? How?

On Sun, Oct 4, 2015, 1:22 PM Jim Cummins notifications@github.com
wrote:

Thanks @accraze https://github.com/accraze!!! Merged! Also thanks
to
@kentcdodds https://github.com/kentcdodds for merge and rebase
which
let me merge from my phone!


Reply to this email directly or view it on GitHub
<
#9 (comment)
.

Reply to this email directly or view it on GitHub
<#9 (comment)
.


Reply to this email directly or view it on GitHub
#9 (comment).

Reply to this email directly or view it on GitHub
#9 (comment).

@joshmanders
Copy link

Sorry to bump this, but is there a reason that it's got it's own object, czConfig, instead of the universal config object? https://docs.npmjs.com/files/package.json#config

@kentcdodds
Copy link
Member

+1. I don't think there's a reason @killswitch except perhaps ignorance (there's so much to know!) I vote that the next release looks for config there first, and then falls back on czConfig and gives a deprecation warning. Then in 3.0.0, we no longer look at czConfig. What do you think @jimthedev?

@jimthedev
Copy link
Member

@killswitch That is a good idea. I think the main reason is that the config loader originally came from jscs and thus followed their perhaps naive convention. The good news is we can easily add it and migrate to it being the default.

Things needed for this to happen:

  • Add config.cz as the highest level priority option in the configLoader (note, we still need to support czConfig for now, but can add a deprecation warning in the cli to get people to switch)
  • Change commitizen init to use the config.cz location by default to encourage new adoption.
  • Change the docs to reflect this change
  • Add deprecation warning to cli, potentially even put in PRs on gh.
  • Eventually remove support for czConfig in 3.0.0 per @kentcdodds

Seem like a good plan?

@joshmanders
Copy link

Perfect. You guys are awesome. :)

@jimthedev
Copy link
Member

@killswitch Thanks for keeping us honest. 👍

@joshmanders
Copy link

No problem @jimthedev. Thank you guys for a great module. I'm implementing it on all my projects thanks to @kentcdodds video on egghead.io

@jimthedev
Copy link
Member

Good to hear. Cheers!
On Mon, Oct 19, 2015 at 2:23 PM Josh Manders notifications@github.com
wrote:

No problem @jimthedev https://github.com/jimthedev. Thank you guys for
a great module. I'm implementing it on all my projects thanks to
@kentcdodds https://github.com/kentcdodds video on egghead.io


Reply to this email directly or view it on GitHub
#9 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants