Skip to content
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

Feature fetch upstream and several bug fixes #64

Conversation

RobertReaves
Copy link
Collaborator

@RobertReaves RobertReaves commented Jul 11, 2017

Short description of the work completed

Fixed an issue that caused tag-release to only fetch changes in master instead of on the whole upstream remote. There was also an issue with some commands for Windows users when using single quotes instead of double quotes, this has now been addressed. Used to have a -V, --version command for tag-release that allow the user to see the current version they were running. This broken at some point in time and it is now working again. Lastly, updated the README.md file for new features.

Steps to test (if not obvious)

  1. Do a normal tag-release flow by releasing a change in develop as a full release. This should now properly fetch all changes in upstream instead of just master. You can test this by pushing a change to the develop branch and then attempting to tag-release from develop without having the changes you just pushed.
  2. Attempt to do a normal tag-release from Windows. Should work as intended.
  3. run tag-release -V and tag-release --version, you should now properly see the current version you are running of tag-release

For Reviewer Use Only

  • Code Reviewed
  • Tests Passed
  • Coverage Reviewed
  • Feature worked as expected
  • Added or updated flags to --help and README.md
  • Does the feature work in Windows PowerShell?
  • Is the version upgrade path clear for this change? (breaking vs minor vs
    patch)
  • Follow up work tracked in a card if needed

README.md Outdated
The `--promote` feature also allows for an optional command-line argument of a tag in the follow form:

```
Usage:
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels a little bit confusing having the Usage and Example. I understand you want to show that the v is optional. Maybe remove the Usage section and have two examples (like you do for the Usage section).

Then either have one of the following...

  • a comment at the end of the 2nd example saying that the v is optional
    $ tag-release --promote 1.1.1-feature.2 # the "v" is optional
  • update the paragraph above the examples saying the v is optional

The --promote feature also allows for an optional command-line argument (v is optional) of a tag in the follow form:

@@ -133,7 +134,7 @@ describe( "git", () => {
expectedRunCommandArgs: { args: "tag --sort=v:refname", logMessage: "Getting list of tags" }
},
shortLog: {
expectedRunCommandArgs: { args: "--no-pager log --no-merges --date-order --pretty=format:'%s'", logMessage: "Parsing git log" }
expectedRunCommandArgs: { args: "--no-pager log --no-merges --date-order --pretty=format:\"%s\"", logMessage: "Parsing git log" }
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please update this to be surrounded by `` and then use non-escaped double quotes inside? I feel that it reads cleaner than escaped characters. Thanks

@@ -194,7 +195,7 @@ describe( "git", () => {
},
generateRebaseCommitLog: {
args: "v1.1.1-blah.0",
expectedRunCommandArgs: { args: "log upstream/master..HEAD --pretty=format:'%h %s' --no-merges" }
expectedRunCommandArgs: { args: "log upstream/master..HEAD --pretty=format:\"%h %s\" --no-merges" }
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

it( "should call `git.shortLog` with the appropriate options when a tag is given", () => {
return git.shortLog( "v1.2.3" ).then( () => {
expect( git.runCommand ).toHaveBeenCalledTimes( 1 );
expect( git.runCommand ).toHaveBeenCalledWith( { args: "--no-pager log --no-merges --date-order --pretty=format:'%s' v1.2.3..", logMessage: "Parsing git log" } );
expect( git.runCommand ).toHaveBeenCalledWith( { args: "--no-pager log --no-merges --date-order --pretty=format:\"%s\" v1.2.3..", logMessage: "Parsing git log" } );
Copy link
Contributor

Choose a reason for hiding this comment

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

and here :)

Copy link
Contributor

@elijahmanor elijahmanor left a comment

Choose a reason for hiding this comment

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

Please take a look at the requested changes

@elijahmanor elijahmanor merged commit 1812121 into LeanKit-Labs:feature-fetch-upstream Jul 11, 2017
@RobertReaves RobertReaves deleted the feature-fetch-upstream branch July 16, 2018 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants