-
Notifications
You must be signed in to change notification settings - Fork 83
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
Create new submodule update target to run git submodule update #61
base: master
Are you sure you want to change the base?
Changes from 4 commits
928a463
1824a5c
2e205ac
4b5f632
0f0b8f6
e3380fd
8984b98
4f22223
384bfaf
1278809
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
'use strict'; | ||
|
||
var async = require('grunt').util.async; | ||
var grunt = require('grunt'); | ||
|
||
module.exports = function (task, exec, done) { | ||
var optionKey; | ||
var allowedOptions = { | ||
init: false, | ||
remote: false, | ||
force: false, | ||
rebase: false, | ||
merge: false, | ||
reference: null, | ||
recursive: false, | ||
}; | ||
|
||
var options = task.options(allowedOptions); | ||
|
||
var args = ['submodule', 'update']; | ||
|
||
|
||
// Loop through allowable cli flags in options and add to args | ||
for (optionKey in allowedOptions) { | ||
if (options.hasOwnProperty(optionKey) && options[optionKey]) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. I had to do a little research to confirm, but it looks like my understanding of property enumeration was very outdated. After ES5, it is quite simple to mark prototype properties as non-enumerable, so adding https://groups.google.com/forum/#!topic/jsmentors/2kmsNxOirFk |
||
// Add flag | ||
args.push('--' + optionKey); | ||
// If not a boolean, add the value after the flag | ||
if (typeof options[optionKey] !== 'boolean') { | ||
args.push(options[optionKey]); | ||
} | ||
} | ||
} | ||
|
||
// If a path was specified, add it now: | ||
if (options.path) { | ||
args.push(options.path); | ||
} | ||
|
||
// Special handling is also needed for no-fetch option because | ||
// of hyphen in flag: should not be used as property name | ||
if (options.noFetch) { | ||
args.push('--no-fetch'); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See the note on case above, we can probably do this automatically with some regex magic. Something like this: "noFetch".replace(/\W+/g, '-').replace(/([a-z\d])([A-Z])/g, '$1-$2').toLowerCase() -> "no-fetch" |
||
|
||
// Depth also needs to be specified explicitly here | ||
// because a depth of zero would be skipped in the loop | ||
if (options.depth !== null && options.depth !== undefined) { | ||
args.push('--depth'); | ||
args.push(options.depth); | ||
} | ||
|
||
// Add callback | ||
args.push(done); | ||
|
||
exec.apply(this, args); | ||
}; | ||
|
||
module.exports.description = 'Update git submodules.'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
'use strict'; | ||
|
||
var command = require('../lib/commands').submoduleupdate; | ||
var Test = require('./_common'); | ||
|
||
describe('submodule update', function () { | ||
it('should update submodules', function (done) { | ||
var options = { | ||
}; | ||
|
||
new Test(command, options) | ||
.expect(['submodule', 'update']) | ||
.run(done); | ||
}); | ||
|
||
it('should accept init option', function (done) { | ||
var options = { | ||
init: true | ||
}; | ||
|
||
new Test(command, options) | ||
.expect(['submodule', 'update', '--init']) | ||
.run(done); | ||
}); | ||
|
||
it('should accept remote option', function (done) { | ||
var options = { | ||
remote: true | ||
}; | ||
|
||
new Test(command, options) | ||
.expect(['submodule', 'update', '--remote']) | ||
.run(done); | ||
}); | ||
|
||
it('should accept no-fetch option', function (done) { | ||
var options = { | ||
noFetch: true | ||
}; | ||
|
||
new Test(command, options) | ||
.expect(['submodule', 'update', '--no-fetch']) | ||
.run(done); | ||
}); | ||
|
||
it('should accept force option', function (done) { | ||
var options = { | ||
force: true | ||
}; | ||
|
||
new Test(command, options) | ||
.expect(['submodule', 'update', '--force']) | ||
.run(done); | ||
}); | ||
|
||
it('should accept rebase option', function (done) { | ||
var options = { | ||
rebase: true | ||
}; | ||
|
||
new Test(command, options) | ||
.expect(['submodule', 'update', '--rebase']) | ||
.run(done); | ||
}); | ||
|
||
it('should accept merge option', function (done) { | ||
var options = { | ||
merge: true | ||
}; | ||
|
||
new Test(command, options) | ||
.expect(['submodule', 'update', '--merge']) | ||
.run(done); | ||
}); | ||
|
||
it('should accept reference option', function (done) { | ||
var options = { | ||
reference: 'https://myrepo.com/repo.git' | ||
}; | ||
|
||
new Test(command, options) | ||
.expect(['submodule', 'update', '--reference', 'https://myrepo.com/repo.git']) | ||
.run(done); | ||
}); | ||
|
||
it('should accept depth option', function (done) { | ||
var options = { | ||
depth: 10 | ||
}; | ||
|
||
new Test(command, options) | ||
.expect(['submodule', 'update', '--depth', '10']) | ||
.run(done); | ||
}); | ||
|
||
it('should accept recursive option', function (done) { | ||
var options = { | ||
recursive: true | ||
}; | ||
|
||
new Test(command, options) | ||
.expect(['submodule', 'update', '--recursive']) | ||
.run(done); | ||
}); | ||
|
||
it('should accept path option', function (done) { | ||
var options = { | ||
path: '/test/path' | ||
}; | ||
|
||
new Test(command, options) | ||
.expect(['submodule', 'update', '/test/path']) | ||
.run(done); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where I feel like I've done things differently than you would have, @rubenv ;-)
If you prefer, I am happy to change this to explicitly pushing individual args onto the args array, rather than doing so in a loop.
Notice that we are looping through
allowedOptions
, notoptions
. This will ensure that only the supported options are added to the args array.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it's cool, it cuts back on the amount of lines. Might be a good future improvement to convert all binary options everywhere to a logic similar to this.
We'd need to handle dashes:
no-merge
->noMerge
.