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

Refactoring #84

Open
gabyx opened this issue Feb 7, 2022 · 8 comments
Open

Refactoring #84

gabyx opened this issue Feb 7, 2022 · 8 comments

Comments

@gabyx
Copy link
Contributor

gabyx commented Feb 7, 2022

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
Copy link
Contributor Author

gabyx commented Feb 7, 2022

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
Copy link
Contributor Author

gabyx commented Feb 7, 2022

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

@axetroy
Copy link
Member

axetroy commented Feb 7, 2022

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
Copy link
Member

axetroy commented Feb 7, 2022

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~

@gabyx
Copy link
Contributor Author

gabyx commented Feb 8, 2022

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
Copy link
Contributor Author

gabyx commented Feb 8, 2022

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

@gabyx
Copy link
Contributor Author

gabyx commented Feb 9, 2022

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
Copy link
Contributor Author

gabyx commented Feb 9, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants