whatchanged icon indicating copy to clipboard operation
whatchanged copied to clipboard

Refactoring

Open gabyx opened this issue 4 years ago • 8 comments

It might be nice, if the revision range just supports Git syntax "SHA..SHA". The parsers seems a bit complicated. Actually the whole thing could be made much easier:

git rev-parse A..B

is extremely powerfull and could be used once, to get the list of commits. Then another for loop just parses everything.

Thats gonna result in a 5% of the code in 0_parser.go And I really suggest to use open ranges https://github.com/gabyx/Githooks/blob/06b29e49f6bf62b58bee38431eb3603e45bb85f2/githooks/git/gitcommon.go#L313 with a maybe flag --include-last.

Do you think that might work?

gabyx avatar Feb 07 '22 13:02 gabyx

Actually, we could just drop most features (eventough they seem nice, are they really needed? @N etc..) You certainly almost always know from where to where you genereate the log. :-) I would just support the git syntax, meaning, one would forward it directly to git rev-parse -> Done. :-)

gabyx avatar Feb 07 '22 13:02 gabyx

Why I come with this: there is no way to generate the log from HEAD to lastTag (without the Tag!)...

gabyx avatar Feb 07 '22 13:02 gabyx

eventough they seem nice, are they really needed? @n etc..

This seems to make sense.

But it does have some scenarios, eg. generating the changelog of the last two versions from HEAD, HEAD~@2

axetroy avatar Feb 07 '22 17:02 axetroy

Why I come with this: there is no way to generate the log from HEAD to lastTag (without the Tag!)...

It should work without any tags. We can add a test case to fix it.

try run command:

whatchanged --project=https://github.com/release-lab/test-first-commit.git --preset=full HEAD~

axetroy avatar Feb 07 '22 17:02 axetroy

Sorry I was not clear: I meant from HEAD to first next tag... (not including the tag). Say HEAD contains v1.4.0 and after 3 commits v1.3.9 comes...

because closed ranges are used, this is trouble some. Therefore (and in many other languages, its better to use open ranges.)

gabyx avatar Feb 08 '22 19:02 gabyx

Ok it seams that just works, by not giving any arguments.

gabyx avatar Feb 08 '22 19:02 gabyx

Ok, strange things happen when some tag is somewhere else (not seen from HEAD)

The behavior you certainly want is to replace your current parsing with git rev-parse --first-parent with the arguments

  • A..B (specifying a commit range, if not given its just HEAD)
  • --stop-at-nth-tag 2

Then collecting all SHA commits, and then hand them over to parse.

In that way the user has more possibilities and the result is also more correct. Do you indent to implement this?

gabyx avatar Feb 09 '22 12:02 gabyx

Ok, hm... its not that easy I guess. When tags can be anywhere: One want certainly everything between version tags... So just traversing the DAG with rev-list might not work with a sole range

  • Better is to get all version tags, sorted greatest to lowest into L.
  • Add HEAD at the top (if specified by input)
  • Then use sucecssively ancestry paths:
    commits = []
    foreach tagS in L:
      tagE = prev(L, tagS)
      cs = git rev-list --first-parent --ancestry-path tagE..tagS
                // where `tagE < tagS` 
      if(cs is empty):
          error out... because something is fishy: tagE cannot be reached from tagS.
      commits.append(cs)
    

So I guess the syntax A~B you are using can still be used. With the change, that B is not yet included when --no-last-tag or something like that. So that said, I would only provide the following options, first some notes:

  • S--->E means the ancestrypath from start S to end E.
  • @X corresponds to the X-th in the list of version tags.
  • Ranges with ~ boil down into version tag ranges, see below
  • Ranges with .. are simple git rev-list *ancestry-paths ranges and commits get collected that way and then spliced into segments according to the found tags in the commit list.

With Tags:

  1. @1~@0 : all version tags between @0 and @1 then always the ancestry path between them e.g. v0.0.0--->v1.0.0--->v2.0.0). Note: If e.g. v1.0.0--->v2.0.0 is not reachable, print a warning, and generate nothing for verison v2.0.0. Because its strange.

  2. @3~ : transforms to @3~HEAD = @3~@0 + @0--->HEAD, then the ancestry path between them (see 1.) Note: @0--->HEAD could be empty if @0 not reachable from HEAD

  3. v1.0.0~v2.0.1 : all version tags between v1.0.0 and v2.0.1, then always the ancestry path between them e.g. v1.0.0--->v1.1.0--->v2.0.1 (see 1.)

  4. ~v1.0.0 : transforms to InitCommit~v1.0.0 = InitCommit--->@N + @N-1~v1.0.0 . (see 1.)

  5. v1.0.0~ef34123 (or @10~ef34123) : transforms to v1.0.0~@X + @X--->ef34123. (see 1.) Commit ef34123 acts as an abort marker where @X is some tag in L after v1.0.0, possibly @X could end up to be @0 and the same note applies as in 1.!

  6. ef34123~v1.0.0 (or ef34123~@3) : transforms to ef3412--->@X + @X~v1.0.0. (see 1.) Commit ef34123 acts as an abort marker where @X is some tag in L. , possibly @X could end up to be @N and the same note applies as in 1.!

  7. ef34123~123aef : No dont support this. Use the thing below.

With Commits:

It gets tricky, dont use the ~ syntax, because that would suggest we could get all tags between a commit range (which is not clear in a general DAG (release branches which have patch versions etc...), but that only makes sense talking about the direct ancestry-path from start to end... so therefore directly use Gits syntax

  • ef123a..123aef : Gather all tags between these commits, store in L. Use git rev-list --first-parent --ancestry-path ef123a..123aef1. Splice the list L into your splices according to the version tags discovered.

Options --no-last-splice will exclude the last splice, makeing it possible to have open ranges for everythin above.

invocation

Then there is more difficulties with invocation:

whatchanged    ef34123~v1.0.0    v0.0.1    af312..afeb3   v3.0.0~v4.0.0
// Group 1: ---#######A######---###B###
// Group 2: -------------------------------######C#####
// Group 3: ----------------------------------------------####D####                          

All splices in group 1 are outputted first (note B must come first), than group 2, than group 3. Only like that it makes sense or you safe the hassle and say its outputted as stated on the command line... :-)

gabyx avatar Feb 09 '22 20:02 gabyx