-
Notifications
You must be signed in to change notification settings - Fork 6
Add version option #10
base: master
Are you sure you want to change the base?
Conversation
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.
Great idea! A little too npm
specific for my liking, but I'm sure that can be amended later. Look into my comments and address the git push
issue and this should be good to merge and release. :)
var json = JSON.parse(packageContent.toString()); | ||
var version = json.version; | ||
gutil.log(gutil.colors.yellow('Tag version ' + version)); | ||
var cmdTag = spawn('git', ['tag', '-a', 'v' + version, '-m', 'Version ' + version], {cwd: repoPath}); |
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.
I'd prefer the 'v' + version
and 'Version ' + version
strings to be configurable (maybe via a lambda call?), but that can be fixed in a later PR.
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.
Configurable is good, but I'm just make quick to reach my needs.... 😁
index.js
Outdated
return callback(null); | ||
}); | ||
}, | ||
function gitPushVersion(callback) { |
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.
Can't the versioned tag be pushed out with the rest of the repository? Shouldn't be a need for multiple git push
calls. Do correct me if I'm missing something though.
Since this is also located before the gitPush
function, I'm pretty sure this will break usage of the remoteBranch
option and push to the default branch regardless.
} | ||
return callback(null); | ||
}); | ||
}, | ||
function gitPush(callback) { | ||
gutil.log(gutil.colors.yellow('Pushing to remote deployment repository')); | ||
var cmdPush = spawn('git', ['push', 'origin', options.remoteBranch], {cwd: repoPath}); |
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.
You should be able tomake this var cmdPush = spawn('git', ['push', 'origin', options.remoteBranch, '--tags'], {cwd: repoPath});
to fix the git push
comment above. Does require testing to make sure it works though.
index.js
Outdated
@@ -201,6 +202,52 @@ module.exports = function(options) { | |||
return callback(null); | |||
}); | |||
}, | |||
function version(callback) { |
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.
Following the convention used for the other method signatures, this should be named gitTag
, gitTagVersion
or similar.
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.
👌 got it.
index.js
Outdated
}, ''); | ||
var packageContent = fs.readFileSync(packageJson, 'utf8'); | ||
var json = JSON.parse(packageContent.toString()); | ||
var version = json.version; |
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.
Could the version extraction be moved into it's own support function? Would make it easier to support other project types later.
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.
Just updating the changeset with one of the comments that went outdated. It's also the one issue that actually needs to be resolved before I can merge and release.
return callback(null); | ||
}); | ||
}, | ||
function gitPushTag(callback) { |
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.
Attaching this comment to the new changeset since it turned outdated on the rename:
Can't the versioned tag be pushed out with the rest of the repository? Shouldn't be a need for multiple git push calls. Do correct me if I'm missing something though.
Since this is also located before the gitPush function, I'm pretty sure this will break usage of the remoteBranch option and push to the default branch regardless.
gutil.log(gutil.colors.yellow('Pushing version to remote deployment repository')); | ||
var cmdPush = spawn('git', ['push', 'origin', '--tags'], {cwd: repoPath}); | ||
cmdPush.stderr.on('data', function(data) { | ||
if (options.verbose || options.debug) gutil.log(gutil.colors.magenta('git push verion: ') + data.toString().trim()); |
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.
Typo here: verion
if (options.verbose || options.debug) gutil.log(gutil.colors.magenta('git push verion: ') + data.toString().trim()); | ||
}); | ||
cmdPush.stdout.on('data', function(data) { | ||
if (options.verbose || options.debug) gutil.log(gutil.colors.magenta('git push verion: ') + data.toString().trim()); |
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.
Typo here: verion
}); | ||
cmdPush.on('close', function(code) { | ||
if (code !== 0) { | ||
return callback('git push verion exited with code ' + code); |
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.
Typo here: verion
git tag version from
package.json
version field.