Updates

[jzaefferer]

This PR mixes two fixes and two unrelated changes, though each commit is atomic and should be easy to review. I'm doing a single PR to ping some people just once, please take a look: @mislav (for the #22 fix), @gibson042 (for the #23 fix) and @scottgonzalez. Thanks!

[scottgonzalez]

This looks good.

[jzaefferer]

I've added two commits, one fixing the missing space (will squash later), the other introducing lib/sanitize.js to clean up the commit message before validating it. This still assumes "#" is used as comment char. There's probably some way to retrieve the current config value, while falling back to the default, but that seems like a lot of effort for something that I've never heard of anyone making use of.

[gibson042]

Looking at this again, it may be better to separate "component required" from "list of valid components" (e.g., component: true vs. components: [ "Build", "Test", "Core" ]), perhaps for hypothetical situations where component is optional but must come from the list if specified. I don't think it matters very much (certainly not with respect to jQuery), but thought it was worth raising for consideration.

[jzaefferer]

@gibson042 I've implemented that change. Makes the code a bit cleaner, too. And makes it easier to access the list for other purposes, like the one suggested in #25.

[mislav]

On Tue, Oct 7, 2014 at 2:34 PM, Jörn Zaefferer notifications@github.com
wrote:

This still assumes "#" is used as comment char. There's probably some way
to retrieve the current config value, while falling back to the default,
but that seems like a lot of effort for something that I've never heard of
anyone making use of.

It's a core git feature, and I think it's a good idea to support these
features lest you risk your project to be broken for people with different
kinds of configurations.

You should just shell out to git config core.commentchar and default to
"#" if it's blank.

[jzaefferer]

You should just shell out to git config core.commentchar and default to
"#" if it's blank.

To do that synchronously in node I currently need to add a dependency on exec-sync, which has a compile step when installing. It also has a bunch of dependencies itself, both slow down the installation of this module a lot. I'll create a separate issue to address that issue at some point in the future, either when node has sync exec or someone actually runs into that problem. See #27.

I'll land this as-is for now and publish.

[scottgonzalez]

FYI: execSync() exists in node 0.11.