Skip to content

Ignore revisions from #1307 in git blame #1310

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 4, 2023
Merged

Ignore revisions from #1307 in git blame #1310

merged 1 commit into from
Dec 4, 2023

Conversation

berquist
Copy link
Member

@berquist berquist commented Dec 2, 2023

Closes #1122

I did everything as outlined in https://www.michaelheap.com/git-ignore-rev/, though one may not want to set a global file:

git config [--global] blame.ignoreRevsFile .git-blame-ignore-revs
# Mark any lines that have had a commit skipped using --ignore-rev with a `?`
git config --global blame.markIgnoredLines true
# Mark any lines that were added in a skipped commit and can not be attributed with a `*`
git config --global blame.markUnblamableLines true

And here are some examples of the output:

*06e54bd1 cclib/__init__.py     (Eric Berquist    2023-12-02 09:29:14 -0500 24) # isort: off
?4d22f3a2 src/cclib/__init__.py (Eric Berquist    2017-12-12 11:33:08 -0500 25) from cclib import parser, progress, method, bridge, io
*06e54bd1 cclib/__init__.py     (Eric Berquist    2023-12-02 09:29:14 -0500 26) # isort: on

where I think the isort directives show as * because they are only from an ignored commit. The same appears for lines with isolated parens that were not previously on their own lines (see qchemparser.py and grep for 2023-12).

A slightly more confusing case is adfparser.py, but the lines from that date are all whitespace and correspond to the CRLF -> LF commit, and those changes seem to constitute a complete change rather than reuse whatever empty line was there before. But, they're not code lines.

@berquist berquist added this to the v1.8.1 milestone Dec 2, 2023
@berquist
Copy link
Member Author

berquist commented Dec 2, 2023

@shivupa and @amandadumi , can one of you look at the git blame output of your favorite file after doing at least git config blame.ignoreRevsFile .git-blame-ignore-revs and see if it makes sense to you?

@amandadumi
Copy link
Member

I think this looks good and seems to have worked. In a quick/narrow check, the nwchem parser went from seeing your name on every line of the git blame to only on what looks like newly added lines, which seems correct to me.

@shivupa
Copy link
Member

shivupa commented Dec 4, 2023

+1 LGTM TY

@shivupa
Copy link
Member

shivupa commented Dec 4, 2023

A slightly more confusing case is adfparser.py, but the lines from that date are all whitespace and correspond to the CRLF -> LF commit, and those changes seem to constitute a complete change rather than reuse whatever empty line was there before. But, they're not code lines.

I think as long as the code lines are mostly git blame-able we are in business.

@shivupa shivupa merged commit ceb096c into cclib:master Dec 4, 2023
@berquist berquist deleted the 1122-git-blame-ignore-revs branch December 4, 2023 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shivupa shivupa shivupa approved these changes

@amandadumi amandadumi Awaiting requested review from amandadumi

Assignees
No one assigned
Labels
maintenance
Projects
None yet
Milestone
v1.8.1
Development

Successfully merging this pull request may close these issues.

Should we have some sort of code style?
3 participants
@berquist @amandadumi @shivupa