search

spacedentist / spr

Submit pull requests for individual, amendable, rebaseable commits to GitHub
https://getcord.github.io/spr/
MIT License
377 stars 33 forks source link

Commit in PR shows as "[spr] Initial commit" #130

Closed fredrikaverpil closed 1 year ago

fredrikaverpil commented 1 year ago

Hi 👋 😄

I found myself with a large PR and wanted to give spr a try to see if I could make it easier to manage it and give my colleagues a better experience in review.

I followed the getting started guide and added a simple commit on the main branch of my pre-existing project (with lots of pre-existing commits). When I then ran spr diff a GitHub PR was created 👍 but the commit looks like this:

image

If I run git log locally I can see the actual commit message I was expecting.

I feel this is a problem for me, as I don't want to force the spr workflow onto anyone in my team and in case they would merge this PR it would get merged with the wrong commit message.

Is this expected behavior?

sven-of-cord commented 1 year ago

Hi @fredrikaverpil !

This is expected behaviour. When you update an existing PR by calling spr diff on it again, it will ask you for an update message. Something that describes to your reviewers what changes/fixes/improvements you have made in this update. This makes for a nice timeline on GitHub, where you can see people commenting on your code, e.g. "you have a typo here", followed up by an update of your code titled "fix typo", etc. etc.

When your PR is ready to merge, you can use spr land to do that, and it will squash-merge your PR onto the main branch, so it becomes all a single commit again (exactly the commit you had locally), and that will automatically have the proper commit message.

The assumption here is that as the author of a PR, you will be the one who merges it, not somebody else. So it's okay if you're the only spr user in your team, nobody else has to use it. But the sad truth is that GitHub displays a merge button, and it doesn't produce the commit message we want, so you have to resist using that and use spr land instead. (Or you copy and paste the right message into the text field manually every time - but who wants to do that?) The added benefit of using spr land for landing is that it will also complain if you made changes locally that you forgot to upload. spr land only does the merge if the PR is still in sync with your local commit, for peace of mind. If it isn't, it will tell you to do another spr diff first - after which you can see on GitHub what additional changes there were. (If you're interested in technical detail: spr land checks if cherry-picking your local commit onto master produces the same result as merging the PR in its current state - only if it does, it proceeds with the API request to GitHub to squash-merge the PR.)

fredrikaverpil commented 1 year ago

Ok, thanks for the explanation @sven-of-cord 👍

I appreciate the hard work done on spr but I ended up having graphite.dev fit my needs better, so I'm using that instead.