Show the lastTag and the noOfAdditionalCommits #37
Conversation
index.js
Outdated
|
|
||
| var lastTagInfo = findLastTag(gitPathInfo.commonGitDir, result.sha); | ||
| result.lastTag = lastTagInfo.tag; | ||
| result.noOfAdditionalCommits = lastTagInfo.noOfAdditionalCommits; |
There was a problem hiding this comment.
maybe commitsSinceLastTag would be more clear?
index.js
Outdated
| @@ -149,6 +149,26 @@ function findTag(gitPath, sha) { | |||
| return tags.length ? tags[0] : false; | |||
| } | |||
|
|
|||
| function findLastTag(gitPath, sha, noOfAdditionalCommits) { | |||
| noOfAdditionalCommits = noOfAdditionalCommits || 0; | |||
| var tag = findTag(gitPath, sha); | |||
There was a problem hiding this comment.
This is going to read and scan .git/packed-refs for each commit from HEAD to whatever has a tag, or all commits if there are no tags.
If we're going to always run findLastTag, we may want to cache packed-refs. As I recall git-repo-info winds up getting called a lot in certain builds. Is that right @rwjblue?
| second commit without tag | ||
| # Please enter the commit message for your changes. Lines starting | ||
| # with '#' will be ignored, and an empty message aborts the commit. | ||
| # On branch master |
There was a problem hiding this comment.
i don't think you intended to include this file
| bare = false | ||
| logallrefupdates = true | ||
| ignorecase = true | ||
| precomposeunicode = true |
There was a problem hiding this comment.
i don't think you intended to include this file
| @@ -0,0 +1 @@ | |||
| Unnamed repository; edit this file 'description' to name the repository. | |||
There was a problem hiding this comment.
i don't think you intended to include this file
| @@ -0,0 +1,2 @@ | |||
| 0000000000000000000000000000000000000000 e66f7ec2da3b5d06f0fe845c4fbc87247efacf62 Lukas Kohler <[email protected]> 1507946495 +0200 commit (initial): first commit | |||
| e66f7ec2da3b5d06f0fe845c4fbc87247efacf62 fb26504da0ed5cd9ed366f7428c06a8433fd76e6 Lukas Kohler <[email protected]> 1507946563 +0200 commit: second commit without tag | |||
There was a problem hiding this comment.
i don't think you intended to include this file
| @@ -0,0 +1,2 @@ | |||
| 0000000000000000000000000000000000000000 e66f7ec2da3b5d06f0fe845c4fbc87247efacf62 Lukas Kohler <[email protected]> 1507946495 +0200 commit (initial): first commit | |||
| e66f7ec2da3b5d06f0fe845c4fbc87247efacf62 fb26504da0ed5cd9ed366f7428c06a8433fd76e6 Lukas Kohler <[email protected]> 1507946563 +0200 commit: second commit without tag | |||
There was a problem hiding this comment.
i don't think you intended to include this file
| @@ -0,0 +1,2 @@ | |||
| x��A | |||
| �0E]���&i"��I:��i#���V=����?~*�: h�OR����fH�t���h����k���L I�&S��lK��Q����C��{�d��r�X�T�\ �,ygԈ��9���8�]�WTo�F= | |||
There was a problem hiding this comment.
i don't think you intended to include this file
| @@ -0,0 +1,2 @@ | |||
| x��A��0 Eg�Sx��<�qS !�p��q���A�ק� a��K��I������e�*P�>a�Ys��Q� �>ƚU�Gr���l�̵S�% r(��F | |||
| B5K�<uZ�T�.�6���==�҆Q8� ����f4��>�4��M'���ā�C��u ���o�{����^��p5����%&Y� | |||
There was a problem hiding this comment.
i don't think you intended to include this file
There was a problem hiding this comment.
actually this file is required!
| commitMessage: 'second commit without tag', | ||
| root: repoRoot, | ||
| parents: [ | ||
| 'e66f7ec2da3b5d06f0fe845c4fbc87247efacf62' |
There was a problem hiding this comment.
maybe add a test to handle the merge commit case (ie multiple parents) since it looks like you already added code to make that work. What do you think?
There was a problem hiding this comment.
I've added a test for the simple case where just one path has a tag. However its not clear what should happen if both paths have a tag (on the same distance). However I dont think we have to figure out all theese edge cases. For now I think its OK to not guarantee a specific behavior in that case.
There was a problem hiding this comment.
@luxferresum yes I agree it's really not clear what the right answer is there.
|
thanks for working on this @luxferresum; LMK if any of my comments are unclear |
luxzeitlos
force-pushed
the
lastTag
branch
2 times, most recently
from
1226433 to
bf078ac
Compare
|
Thanks for the review @hjdivad! I hope I was able to address all your concerns. |
| commitMessage: 'second commit without tag', | ||
| root: repoRoot, | ||
| parents: [ | ||
| 'e66f7ec2da3b5d06f0fe845c4fbc87247efacf62' |
There was a problem hiding this comment.
@luxferresum yes I agree it's really not clear what the right answer is there.
|
@rwjblue tests are failing due to arrow fn. @luxferresum could fix, or we could bump major and drop support for ancient node. your preference? (you already know mine...) |
|
Lets update |
|
awesome, thanks so much for the feature @luxferresum! Looking forward to seeing ember-cli/ember-cli-app-version#49 Looks like a good feature to add 👍 |
|
released as v2.0.0 |
To implement something like
git describeandgit describe --tagswe need not only the tag of the current commit but the tag of the nearest commit in the chain of ancestors with a tag. Additionally we need to know how many additional commits this ancestor is away. This is how it works.This is the basis to solve this issue.