Skip to content
This repository was archived by the owner on Apr 14, 2023. It is now read-only.

Add version option#10

Open
visioncan wants to merge 3 commits into
zhevron:masterfrom
visioncan:master
Open

Add version option#10
visioncan wants to merge 3 commits into
zhevron:masterfrom
visioncan:master

Conversation

@visioncan

Copy link
Copy Markdown

git tag version from package.json version field.

@zhevron zhevron left a comment

Copy link
Copy Markdown
Owner

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. :)

Comment thread index.js
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});

Copy link
Copy Markdown
Owner

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.

Copy link
Copy Markdown
Author

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.... 😁

Comment thread index.js Outdated
return callback(null);
});
},
function gitPushVersion(callback) {

Copy link
Copy Markdown
Owner

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.

Comment thread index.js
},
function gitPush(callback) {
gutil.log(gutil.colors.yellow('Pushing to remote deployment repository'));
var cmdPush = spawn('git', ['push', 'origin', options.remoteBranch], {cwd: repoPath});

Copy link
Copy Markdown
Owner

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.

Comment thread index.js Outdated
return callback(null);
});
},
function version(callback) {

Copy link
Copy Markdown
Owner

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌 got it.

Comment thread index.js
}, '');
var packageContent = fs.readFileSync(packageJson, 'utf8');
var json = JSON.parse(packageContent.toString());
var version = json.version;

Copy link
Copy Markdown
Owner

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.

@zhevron zhevron left a comment

Copy link
Copy Markdown
Owner

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.

Comment thread index.js
return callback(null);
});
},
function gitPushTag(callback) {

Copy link
Copy Markdown
Owner

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.

Comment thread index.js
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());

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo here: verion

Comment thread index.js
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());

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo here: verion

Comment thread index.js
});
cmdPush.on('close', function(code) {
if (code !== 0) {
return callback('git push verion exited with code ' + code);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo here: verion

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants