Hacker News new | past | comments | ask | show | jobs | submit login

This may be a mundane post, but it's sorely needed IMO. Way too many people create crappy commit messages. I've started kicking them back to the developer to fix.



How do you ask them to fix them? I'm imagining that you see the commit message after they've pushed it up somewhere. Are you asking them to reword/amend the commit--i.e., change the history of a pushed branch, with the little bit of extra work that involves?


This isn't really an issue. Contrary to what a lot of people will tell you, it is fine to change the history of a pushed branch -- especially for the most recent commit -- so long as other developers adopt a policy of rebasing after fetch. Here is a concrete example:

(bare repo): # Make sure receive.denyDeleteCurrent = ignore

(Dev A): git push

(Dev B): git pull --rebase # you can make this the default for a particular tracking branch so it is just "git pull"

(Dev B): # do some work

(Dev B): git commit

(Dev A): git commit --amend

(Dev A): git push origin :master # Deletes remote master. Not required for shared branches -- those can simply be replaced -- but "master" is usually the "current branch" for a repo, so it bears knowing.

(Dev A): git push origin master:master # And now we replace the remote's version with our local version.

(Dev B): git pull --rebase # Updates pointers and applies the new commit on top.

Yes, if you do even more devastating history changes than this, Dev B needs to dig a little further on why his rebase is going haywire. THIS is why people fear the history change; it is sometimes extra work for downstream developers to handle the rearrangements (and we developers like to be lazy and for everything to work smoothly the first time). Thankfully, downstream developers can solve these problems with a few git tools, including "rebase --onto" and cherry-picking. But in the case of a quick and simple commit amend, all is well.

Alternatively, you can code review, but sometimes people are in a rush, and you run into this situation. In most environments, I wager it will be okay, especially if there is communication on the changes and reasonable education on version control theory.


Interesting, thanks for the example, works exactly as you said.

There is a race when dev A deletes master though, right? What if someone else made a commit between when dev A last pulled and when dev A deletes master, wouldn't that commit be lost? In the process of testing this I managed to lose dev B's work a couple times, so I still think this kind of operation is too risky to be worthwhile.

In case anyone is interested I made a test script based on Xurinos's example: https://gist.github.com/769904


There is a race when dev A deletes master though, right?

Yes and no. I could say that this is where communication can help a little, and it depends entirely on your local group's policies, but that would be the easy cop-out. Let's say it really happened.

Dev B's git pull --rebase will also lose the change; it was assumed to be pushed up, and a git fetch means that you are reflecting exactly what is in the remote branch. But changes are never truly lost+++. Dev B can check his reflog, find his original commit, and cherry-pick it in, resubmitting it. Afterwards, he can bonk Dev A over the head for not pulling before pushing.

There is a danger that Dev B would not notice, but from a little test I just did, he is going to get some nasty messages during his git pull --rebase that should indicate that something interesting just happened.

I have not looked into this, but does anyone know if it is possible to lock a remote git repo (aside from the automatic push lock)? If one could manually lock the repo, the process would be to lock, pull, push-delete, push-for-real, and unlock.

+++ depending on your git configuration policy. I think you have 30 days before dangling commits are truly pruned permanently.


> I have not looked into this, but does anyone know if it is possible to lock a remote git repo (aside from the automatic push lock)? If one could manually lock the repo, the process would be to lock, pull, push-delete, push-for-real, and unlock.

Or maybe if Git could do a "test and set" on a ref? For example, instead of two step delete and recreate, how about "git push origin +master": forced update, but your Git sends "update master to 1234abcd iff your current master is 5678fedc" where 5678fedc is where origin/master points on your local Git. If someone had snuck in a new commit Git would then fail and you could deal with it. (Of course, on a busy shared repository "deal with it" could take a while if people keep sneaking in commits; but I suspect that kind of commit volume is very rare.)


Oh! I made a mistake. You do not have to delete or use the special config flag if you use --force for your push. I wondered why I remembered doing this on side branches. With --force, we do not have to do this. Also, I think you need to make sure you have -u on this kind of push so that you maintain your tracking connection, but I am not 100% positive.

Don't do this:

    git push origin :master
Do this:

    git push -u --force origin master:master


You do the review before the commit is pushed to the shared public branch. Lots of ways to do this. Here's three:

1. Use format-patch and review via email. 2. git push origin +HEAD:cr/js2

Now another developer can pull that branch and review it before it is merged to master.

3. Use Gerrit.


They push to an intermediate repo, where I can look at the changes. For some projects, we use gerrit, which makes this really easy (it's a review system from google).




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: