10000 add sha to the commit to beam dialog by pradeepmurugesan · Pull Request #4715 · git-town/git-town · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

add sha to the commit to beam dialog #4715

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
Apr 14, 2025
Merged

add sha to the commit to beam dialog #4715

merged 1 commit into from
Apr 14, 2025

Conversation

pradeepmurugesan
Copy link
Member

Fixed #4519

Truncated the sha as the whole sha doesn't look nice. Hard coded the length to 7 as in github.

Can be changed. Kindly let me know your thoughts..

looks like

gt prepend test1 --beam

Select the commits to beam into branch "test1"
0 > [ ] add sha to the commit to beam dialog (1637f07)

@pradeepmurugesan pradeepmurugesan requested a review from kevgo as a code owner April 14, 2025 16:03
@pradeepmurugesan
Copy link
Member Author

Ah I thought CI would kick in on PRs automatically.. 🤔

@kevgo how can I run the CI on PRs ?

@kevgo
Copy link
Contributor
kevgo commented Apr 14, 2025

@pradeepmurugesan thanks so much for this PR, very well done! I especially like how you use already existing helper methods here.

CI didn't kick off as a security mechanism, to prevent crypto mining and theft of secrets by strangers on the internet. I have just kicked off the CI. You can fix the linter error by running make fix. This fixes all auto-fixable issues, including formatting.

@kevgo
Copy link
Contributor
kevgo commented Apr 14, 2025

I have also sent you an invite to join the Git Town organization on GitHub. As a member, you can create branches directly in this repo (no need to work on a forked repo), CI will run automatically, and it also shows a nice Git Town badge on your GitHub profile. Feel free to join, it's also totally okay if you don't want to do that for now. In any case, welcome to the team of Git Town contributors!

@stephenwade stephenwade self-requested a review April 14, 2025 20:13
@pradeepmurugesan
Copy link
Member Author

I have also sent you an invite to join the Git Town organization on GitHub. As a member, you can create branches directly in this repo (no need to work on a forked repo), CI will run automatically, and it also shows a nice Git Town badge on your GitHub profile. Feel free to join, it's also totally okay if you don't want to do that for now. In any case, welcome to the team of Git Town contributors!

Thank you so much.. Have accepted the invite 🙇

Fixed the lint errors.. 👍 Kindly have another look..

@kevgo kevgo merged commit 0ed21d3 into git-town:main Apr 14, 2025
4 checks passed
@kevgo
Copy link
Contributor
kevgo commented Apr 14, 2025

Very awesome, thanks for this contribution! 🙌

@ruudk
Copy link
Contributor
ruudk commented Apr 15, 2025

Thanks @pradeepmurugesan 🥳

@ruudk
Copy link
Contributor
ruudk commented Apr 15, 2025

Would it be an idea to put the sha's first? That's what most Git UI's do too.

@kevgo
Copy link
Contributor
kevgo commented Apr 15, 2025

I'd be okay with that. Given that the SHA is always the same length, it's easy to skip over it.

@stephenwade
Copy link
Member
stephenwade commented Apr 16, 2025 •
edited
Loading

We should also use git rev-parse --short to shorten the hash, instead of hard-coding 7.

  1. Git automatically picks a length for short hashes based on how many objects are in the repository; 7 characters isn't enough to guarantee uniqueness for large repos. For example, in this repo, Git uses 9 characters. See: git/git@e6c587c
  2. This would also cause Git Town to respect the core.abbrev setting.

I was going to suggest just reading the core.abbrev setting, but while looking into it I learned that Git automatically picks a good length if the setting is unspecified, so I think we should just let Git do the work of abbreviating commits.

@stephenwade
Copy link
Member
< 8000 /tr>

@kevgo (By the way, I self-requested a review on this PR yesterday, but it was merged before I had a chance to review. Is there a better way for me to say "please let me take a look at this before merging"?)

@stephenwade stephenwade mentioned this pull request Apr 16, 2025
Merged
@kevgo
Copy link
Contributor
kevgo commented Apr 16, 2025

@stephenwade Thanks for bringing this up. I wasn't sure what you meant by adding yourself as a reviewer and forgot to check in with you. Apologies for that. Traditionally, any of the listed reviewers can merge a PR. I'm happy to adjust this to wait for an explicit review when somebody adds themselves as a reviewer manually.

As a general best practice for such situations, it helps to over-communicate by also adding a comment describing your intent.

This particular pull request is from a first-time contributor. The most important goal for first-time contributors is having a positive experience and building momentum, so I tend to merge their PRs as quickly as possible.

This PR is a solid improvement in functionality, and makes sense as a stand-alone change. Fine-tuning the length of the displayed SHAs specifically for larger repos is a related idea, and imho best addressed as a refinement of this feature in a subsequent PR.

I hope this makes sense. Happy to chat more in a discussion!

@stephenwade
Copy link
Member

No worries, I'm happy to communicate explicitly in the future, and I appreciate your commitment to making new contributors feel welcome.

I've opened #4719 for further refinement. We can continue the discussion about shortening hashes there.

@stephenwade
Copy link
Member

Would it be an idea to put the sha's first? That's what most Git UI's do too.

@ruudk FYI, #4719 includes your suggestion to put the SHA first.

@stephenwade
Copy link
Member

And thank you to @pradeepmurugesan for implementing the first version of this change!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevgo kevgo Awaiting requested review from kevgo kevgo is a code owner

@stephenwade stephenwade Awaiting requested review from stephenwade

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve commit picker (git town prepend --beam)
4 participants
@pradeepmurugesan @kevgo @ruudk @stephenwade
0