-
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 all 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,49 @@ | ||
'use strict'; | ||
|
||
var async = require('grunt').util.async; | ||
var grunt = require('grunt'); | ||
var stringUtil = require('underscore.string'); | ||
|
||
module.exports = function (task, exec, done) { | ||
var optionKey; | ||
var allowedOptions = { | ||
init: false, | ||
remote: false, | ||
force: false, | ||
rebase: false, | ||
merge: false, | ||
reference: null, | ||
depth: null, | ||
recursive: false, | ||
noFetch: false | ||
}; | ||
|
||
var options = task.options(allowedOptions); | ||
|
||
var args = ['submodule', 'update']; | ||
|
||
|
||
// Loop through allowable cli flags in options and add to args | ||
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. 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 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. 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: |
||
for (optionKey in allowedOptions) { | ||
if (options[optionKey] !== undefined && options[optionKey] !== null && options[optionKey] !== false) { | ||
// Add flag | ||
args.push('--' + stringUtil.dasherize(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); | ||
} | ||
|
||
// 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 |
---|---|---|
|
@@ -42,5 +42,8 @@ | |
}, | ||
"keywords": [ | ||
"gruntplugin" | ||
] | ||
], | ||
"dependencies": { | ||
"underscore.string": "~2.3.3" | ||
} | ||
} |
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: 0 | ||
}; | ||
|
||
new Test(command, options) | ||
.expect(['submodule', 'update', '--depth', '0']) | ||
.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 a potential problem: if depth is specified and set to zero, then it will not be set in the loop below (since it will be skipped on line 26:
if (... options[optionKey])
). We can avoid this by changing line 26 to be:Alternatively, we could remove depth from this object, and specify it explicitly down where path and noFetch are specified.
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 has been resolved by explicitly filtering out
undefined
,false
andnull
in the loop below, and allowing0
to pass through.