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

contrib: use specific git log range. Avoid --all. #36

Merged
merged 2 commits into from
May 20, 2022

Conversation

dtrudg
Copy link
Contributor

@dtrudg dtrudg commented May 20, 2022

Because git log --all effectively adds REFS in refs/ to the git log call it will produce output listing commits from all the REFS in our local repo.

Citelang was getting a list of --all commits, and then filtering between start and end points. Because of this, it could pick up commits that are not really in the section of history we are interested in - but come from a fork, or usptream with shared history earlier on.

Call git log with a specific commit range so that it directly provides the correct list of commits we need to consider.

As a bonus, you can now use other refs like branches or deltas as --start and --end points.

Fixes #35

@dtrudg dtrudg marked this pull request as draft May 20, 2022 18:54
@dtrudg dtrudg force-pushed the no-log-all branch 2 times, most recently from 099c18c to 3238697 Compare May 20, 2022 19:11
@dtrudg dtrudg marked this pull request as ready for review May 20, 2022 19:20
citelang/main/contrib.py Outdated Show resolved Hide resolved
citelang/main/contrib.py Outdated Show resolved Hide resolved
Copy link
Owner

@vsoch vsoch left a comment

Choose a reason for hiding this comment

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

okay we are close! A few remaining todos:

I think I found a bug if we specify an end but not start:

$ citelang contrib --end 0.0.13
pip not installed on system, parsing will be done statically (less reliable)
This repository does not have any history.

And I think we probably need to derive the first commit (unless you know some git fu that doesn't require it!)

$ git rev-list --max-parents=0 HEAD
9e915bd2e1ae121a421b5430acda8adf361bd35d
$ git log 9e915bd2e1ae121a421b5430acda8adf361bd35d..0.0.13
# has result

I got that here: https://stackoverflow.com/questions/1006775/how-to-reference-the-initial-commit

And then we need:

  • bump version up in citelang/version.py
  • add entry to CHANGELOG.md to describe change.

and the other combos seem to work ok!

@dtrudg
Copy link
Contributor Author

dtrudg commented May 20, 2022

okay we are close! A few remaining todos:

I think I found a bug if we specify an end but not start:

$ citelang contrib --end 0.0.13
pip not installed on system, parsing will be done statically (less reliable)
This repository does not have any history.

And I think we probably need to derive the first commit (unless you know some git fu that doesn't require it!)

Gotcha - I think I've not really been getting the prior behavior of citelang into my head here, as I'm used to git tools where if a specific start point isn't specified it means 'current commit' rather than beginning of history.

I'll likely be able to get back to this on Monday morning or afternoon. Thanks!

Actually, this'll just be the self.end without the dots. Which will give every commit reachable backward from the specified end.... I'll change that now.

Because `git log --all` effectively adds REFS in `refs/` to the `git
log` call it will produce output listing commits from all the REFS in
our local repo.

Citelang was getting a list of `--all` commits, and then filtering between
start and end points. Because of this, it could pick up commits that
are not really in the section of history we are interested in - but come
from a fork, or usptream with shared history earlier on.

Call `git log` with a specific commit range so that it directly provides
the correct list of commits we need to consider.

As a bonus, you can now use other refs like branches or deltas as `--start`
and `--end` points.

Fixes vsoch#35
@vsoch
Copy link
Owner

vsoch commented May 20, 2022

Sounds good, thanks @dtrudg ! Have a good weekend.

@dtrudg
Copy link
Contributor Author

dtrudg commented May 20, 2022

This should do it now. Thanks for your patience with me :-)

dtrudg@dt-sylabs~/G/citelang> citelang contrib --end 0.0.13
Found 19 commits.
                                           ┏━━━━━━━━━━━━━━━━┳━━━━━━━┓
                                           ┃ Name           ┃ Count ┃
                                           ┡━━━━━━━━━━━━━━━━╇━━━━━━━┩
                                           │ vsoch          │ 3668  │
                                           │ Vanessasaurus  │ 3429  │
                                           │ github-actions │ 1     │
                                           └────────────────┴───────┘
dtrudg@dt-sylabs~/G/citelang> citelang contrib --start 0.0.13
Found 33 commits.
                                           ┏━━━━━━━━━━━━━━━━┳━━━━━━━┓
                                           ┃ Name           ┃ Count ┃
                                           ┡━━━━━━━━━━━━━━━━╇━━━━━━━┩
                                           │ Vanessasaurus  │ 4036  │
                                           │ vsoch          │ 289   │
                                           │ David Trudgian │ 18    │
                                           │ dctrud         │ 7     │
                                           └────────────────┴───────┘
dtrudg@dt-sylabs~/G/citelang> citelang contrib --start 0.0.13 --end 0.0.13
Found 1 commits.
                                            ┏━━━━━━━━━━━━━━━┳━━━━━━━┓
                                            ┃ Name          ┃ Count ┃
                                            ┡━━━━━━━━━━━━━━━╇━━━━━━━┩
                                            │ Vanessasaurus │ 134   │
                                            └───────────────┴───────┘
dtrudg@dt-sylabs~/G/citelang> citelang contrib
Found 52 commits.
                                           ┏━━━━━━━━━━━━━━━━┳━━━━━━━┓
                                           ┃ Name           ┃ Count ┃
                                           ┡━━━━━━━━━━━━━━━━╇━━━━━━━┩
                                           │ Vanessasaurus  │ 7799  │
                                           │ vsoch          │ 3957  │
                                           │ David Trudgian │ 18    │
                                           │ dctrud         │ 7     │
                                           │ github-actions │ 1     │
                                           └────────────────┴───────┘

@dtrudg dtrudg requested a review from vsoch May 20, 2022 21:37
Copy link
Owner

@vsoch vsoch left a comment

Choose a reason for hiding this comment

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

Woohoo! This looks great. Let's merge and release!

@vsoch vsoch merged commit cd9d639 into vsoch:main May 20, 2022
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

Successfully merging this pull request may close these issues.

citelang contrib result depends on local REFs due to git log --all
2 participants