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

Add .git-blame-ignore-revs #3349

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

CasualPokePlayer
Copy link
Member

@CasualPokePlayer CasualPokePlayer commented Aug 8, 2022

This could be used for ignoring giant whitespace changes in git blames.
With this we could commit to doing giant tab/space + newline normalization in .cs files without blame noise. edit: and trailing newlines --yoshi
The commit used here is used more for POC, although still nice for getting rid of the blame noise from it.

This could be used for ignoring giant whitespace changes in git blames.
With this we could commit to doing giant tab/space + newline normalization in .cs files without blame noise.
@YoshiRulz
Copy link
Member

YoshiRulz commented Aug 10, 2022

GitHub doesn't respect this in file history (example). I still like this idea though.

@Morilli
Copy link
Collaborator

Morilli commented Aug 10, 2022

Well, this feature seems to be intended for git blame only; it works here: https://github.com/YoshiRulz/git-blame-ignore-revs-poc/blame/master/src/BizHawk.Client.EmuHawk/EmuHawkUtil.cs

@YoshiRulz YoshiRulz self-requested a review August 15, 2022 16:58
@YoshiRulz
Copy link
Member

YoshiRulz commented Aug 15, 2022

it works here

...or would, if the commit changed any lines in that file.

I've actually changed my mind and am now skeptical of this feature. How exactly does it "see beyond" the filtered commit(s)? Does it only try if the filter commit(s) are whitespace-only? In any case, I'd like to see it in action.

edit: Having reviewed the docs, it seems this has no downsides if we ensure it's only used for trivial whitespace changes. I'm still a bit disappointed that GitHub doesn't use this to glue file history together, but maybe there's another tool that does. GitHub now does that regardless of this file.

some more commits to consider:

@Morilli
Copy link
Collaborator

Morilli commented Aug 16, 2022

https://github.com/Morilli/BizHawk/blame/master/.editorconfig ... I think that's working?

@YoshiRulz
Copy link
Member

Care to revisit this? As per my edits above, I think we should use this feature only for whitespace changes (and file renames).

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

Successfully merging this pull request may close these issues.

4 participants