Summary: | Add "tidy all" commits to a git blame ignore file | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | Pushed to main --- | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | schodkowy.omegi-0r, tomascohen, victor |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | --- | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: |
25.05.00
|
Circulation function: | |
Bug Depends on: | 38664 | ||
Bug Blocks: | 39149 | ||
Attachments: |
Bug 39096: Add 'tidy all' commits to a git blame ignore file
Bug 39096: Add 'tidy all' commits to a git blame ignore file Bug 39096: Add 'tidy all' commits to a git blame ignore file Bug 39096: (follow-up) Fix failing t/Makefile.t test Bug 39096: Add .git-blame-ignore-revs mapping to Makefile.PL |
Description
Jonathan Druart
2025-02-11 15:23:45 UTC
Created attachment 177739 [details] [review] Bug 39096: Add 'tidy all' commits to a git blame ignore file To get a relevant blame we want to exclude some commits from bug 38664. Test plan: 0. Do not apply this patch 1. % git blame C4/Auth.pm (or other file) Notice that there is a lot of reference to bug 38664 (you can see it with the author "Koha Development Team") 2. Apply this patch 3. Tell git to use this new file: % git config blame.ignoreRevsFile .git-blame-ignore-revs Repeat 1. => There are way less refs to "Koha Development Team" Before: % git blame C4/Auth.pm|grep "Koha Development Team" | wc -l 454 After: % git blame C4/Auth.pm|grep "Koha Development Team" | wc -l 29 QA: Do we add the `git config` command that to ktd so devs won't need to do it? Created attachment 178027 [details] [review] Bug 39096: Add 'tidy all' commits to a git blame ignore file To get a relevant blame we want to exclude some commits from bug 38664. Test plan: 0. Do not apply this patch 1. % git blame C4/Auth.pm (or other file) Notice that there is a lot of reference to bug 38664 (you can see it with the author "Koha Development Team") 2. Apply this patch 3. Tell git to use this new file: % git config blame.ignoreRevsFile .git-blame-ignore-revs Repeat 1. => There are way less refs to "Koha Development Team" Before: % git blame C4/Auth.pm|grep "Koha Development Team" | wc -l 454 After: % git blame C4/Auth.pm|grep "Koha Development Team" | wc -l 29 Signed-off-by: Owen Leonard <oleonard@myacpl.org> (In reply to Jonathan Druart from comment #1) > Created attachment 177739 [details] [review] [review] > QA: Do we add the `git config` command that to ktd so devs won't need to do > it? I would think so Yeah I think adding it to ktd would be a nice QoL benefit, I can't think of downsides. But I suggest trying to check if setting it as --global option would work, to avoid changing the configuration of the mounted repo. But even doing it on the level of git repo wouldn't be without advantages, since then the file would be recognized even if someone opens the repo in some IDE outside of ktd! So actually it might be better this way. The only other thing I'm thinking is that some more of old tidy commits could be added here (like a6c9bd0eb55c32d5632625144775271f20aa15f7?). But I'm not sure of a good way to find and identify large commits right now to reasonably find all/most of the past ones, so don't mind it much, could always be added later. Created attachment 178128 [details] [review] Bug 39096: Add 'tidy all' commits to a git blame ignore file To get a relevant blame we want to exclude some commits from bug 38664. Test plan: 0. Do not apply this patch 1. % git blame C4/Auth.pm (or other file) Notice that there is a lot of reference to bug 38664 (you can see it with the author "Koha Development Team") 2. Apply this patch 3. Tell git to use this new file: % git config blame.ignoreRevsFile .git-blame-ignore-revs Repeat 1. => There are way less refs to "Koha Development Team" Before: % git blame C4/Auth.pm|grep "Koha Development Team" | wc -l 454 After: % git blame C4/Auth.pm|grep "Koha Development Team" | wc -l 29 Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> (In reply to Jonathan Druart from comment #1) > QA: Do we add the `git config` command that to ktd so devs won't need to do > it? Yes. As far as I searched, there isn't a default blame.ignoreRevsFile that is picked up. So a `git config` command in KTD to enable it seems reasonable. Tested on a vue file also. Works as expected. The remaining blame lines related to bug 38664 are those that are entirely new from bug 38664. Works, makes sense, QA script happy, code looks good, passing QA :) Pushed for 25.05! Well done everyone, thank you! Looks like we are missing something here: t_Makefile_t.All directories should be mapped: .git-blame-ignore-revs (In reply to Katrin Fischer from comment #9) > Looks like we are missing something here: > t_Makefile_t.All directories should be mapped: .git-blame-ignore-revs Can someone confirm this is the right fix? - Add .git-blame-ignore-revs to MANIFEST.SKIP - Modify t/Make.t to exclude .git-blame-ignore-revs This would be about the same we do for .gitignore if I researched correctly. Created attachment 178144 [details] [review] Bug 39096: (follow-up) Fix failing t/Makefile.t test Ignore .git-blame-ignore-revs for Makefile.t, like we do for .gitignore. (In reply to Katrin Fischer from comment #11) > Created attachment 178144 [details] [review] [review] > Bug 39096: (follow-up) Fix failing t/Makefile.t test > > Ignore .git-blame-ignore-revs for Makefile.t, like we do for .gitignore. This was not correct. Created attachment 178147 [details] [review] Bug 39096: Add .git-blame-ignore-revs mapping to Makefile.PL Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Picking follow-up for main. |