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 gix blame -L start,end #1766

Merged
merged 3 commits into from
Jan 16, 2025
Merged

Conversation

cruessler
Copy link
Contributor

This enables running blame for a portion of a file. This feature will be helful for debugging gix blame, in particular in the context of #1743.

Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot for making blame even better!

Besides my comments which are more like nits to let it appear less rushed, one change I really have to ask for: Changes to gix-blame need to go into their own commit, and be prefixed with feat!: to indicate a breaking change. A single test there probably would be a good idea (or necessity) as well.

The changes to the other two crates can be bundled as they logically are one (core + gitoxide).

Thanks a lot!

gix-blame/src/file/function.rs Show resolved Hide resolved
gix-blame/src/error.rs Outdated Show resolved Hide resolved
gix-blame/src/file/function.rs Outdated Show resolved Hide resolved
src/plumbing/options/mod.rs Outdated Show resolved Hide resolved
src/shared.rs Show resolved Hide resolved
@cruessler
Copy link
Contributor Author

Thanks a lot for making blame even better!

Besides my comments which are more like nits to let it appear less rushed, one change I really have to ask for: Changes to gix-blame need to go into their own commit, and be prefixed with feat!: to indicate a breaking change. A single test there probably would be a good idea (or necessity) as well.

The changes to the other two crates can be bundled as they logically are one (core + gitoxide).

Thanks a lot!

Sure, no worries, and thank for letting me know! I’m not fully familiar with all the conventions yet, so always grateful to learn them. :-)

@cruessler cruessler force-pushed the add-range-to-gix-blame branch from f99eacf to 1f67966 Compare January 14, 2025 14:56
cruessler and others added 2 commits January 16, 2025 08:00
- don't use closures if they con't enclose state (or don't need to)
@Byron Byron force-pushed the add-range-to-gix-blame branch from 1f67966 to 1500c08 Compare January 16, 2025 07:01
@Byron
Copy link
Member

Byron commented Jan 16, 2025

Thanks a lot for the fixes!

Additionally, I have modified the second commits subject to not contain a ! as it's not a breaking change, just a new feature.

@Byron Byron enabled auto-merge January 16, 2025 07:04
@Byron Byron merged commit 90fef01 into GitoxideLabs:main Jan 16, 2025
19 checks passed
@cruessler cruessler deleted the add-range-to-gix-blame branch January 16, 2025 07:53
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.

2 participants