3

I make this mistake quite frequently and I am wondering if there's a better solution, and also whether my current solution is risky.

Here's the workflow

  1. git checkout master
  2. git pull
  3. git submodule update --init --recursive
  4. (edit, debug, test)
  5. git commit -a
  6. git push origin HEAD:refs/for/master
  7. (more edit, test, etc.)
  8. git commit -a
  9. gitk
  10. (swearing)
  11. git rebase -i HEAD~2 # pick and squash my two commits together
  12. git checkout -b task_id
  13. git checkout master
  14. git rebase -i HEAD~2 # pick previous user's commit, delete my own

My mistake is that somewhere around step 5 I should be doing something like step 12 to create a new branch to keep this work separate from every other task, to allow me to easily come back to it or rebase different patches in the desired order or whatever. Instead, I am pushing new code onto the master branch, and around about step 10 I realize that master is no longer pointing at the latest and greatest version of the code that the company has approved, but my own efforts. So after squishing my own commits into one with an interactive rebase, I'm doing another interactive rebase to get master pointed back where it should be -- namely, someone else's work that has been through the review process.

Edit to clarify: When I "push to master" this doesn't publish my changes to everyone else. There's some commit hook magic that creates a Gerrit code review task and only when approval is obtained from that is my commit merged into the remote repository. Apologies for misleading you all. My concern here is only for my local repo.

My question is twofold. How bad is this practice, in terms of risk, and how could it be improved upon? I'm not asking for help in avoiding the initial error. I think I just need to be more careful about which branch I'm pushing to. The question is how best to point master at the previous commit without disturbing/risking/losing anything.

Tim Randall
  • 4,040
  • 1
  • 17
  • 39
  • 3
    "My mistake is that somewhere around step 5 I should be doing something like step 12 " I suggest that at Step 4, before you do any coding at all, that you create a new branch. – Code-Apprentice Oct 23 '18 at 14:53
  • 1
    You can revert a commit, with this you'll point master to the commit that you want but of course you are going to lose every push you have made. Take a look to this document https://help.github.com/desktop/guides/contributing-to-projects/reverting-a-commit/ – Brank Victoria Oct 23 '18 at 14:55
  • @BrankVictoria Note that reverting is different than resetting. Reverting does not lose any previously pushed history because it just creates a new commit on top of the old one with the reverse changes. – Code-Apprentice Oct 23 '18 at 14:59

2 Answers2

6

If you have only pushed a single commit, you can do the following:

git checkout master
git reset --hard HEAD~
git push -f

This will restore master to the previous commit. If you made more commits, you can replace HEAD~ with any other commitish (a SHA1 hash, a branch name, a tag name, etc.).

My mistake is that somewhere around step 5 I should be doing something like step 12

I suggest that at Step 4, before you do any coding at all, that you create a new branch. There should be no ambiguity here. Typical best practice is to create a new feature branch in order to keep on going work separate and only merge it into master once the work is complete. Committing half-complete work on master will make your software unstable and difficult to release.

Code-Apprentice
  • 81,660
  • 23
  • 145
  • 268
  • Please accept my apologies for being unclear in my question. I'm only concerned with the integrity of my local repository, since the Gerrit code review process prevents any modifications to the remote repo. Does that simplify matters here, or make them worse? – Tim Randall Oct 23 '18 at 15:21
  • @TimRandall I was confused by step 6: `git push origin HEAD:refs/for/master`. This appears to push master to the central repo (and is an awfully verbose way of doing so, btw). As for your local repo, technically you can do whatever you want because it is independent of other repos. However, to save your sanity, you should not commit to master. Eventually you will want to pull master from the central repo to stay up to date with the work of other developers. If you commit to master, you will increase the likelihood of merge conflicts which should never happen on master. – Code-Apprentice Oct 23 '18 at 15:38
  • Yep, that's the syntax we're given. I don't know how our code review system works exactly, but that command hooks into it, sending the commit(s) to Gerrit for review and later merging with the remote master. – Tim Randall Oct 23 '18 at 15:59
  • I'm accepting this answer because I think the `git reset` syntax above does exactly what I want, namely pointing `master` back at the parent commit. – Tim Randall Oct 23 '18 at 16:00
1

I am pushing new code onto the master branch, and around about step 10 I realize that master is no longer pointing at the latest and greatest version of the code that the company has approved, but my own efforts.

The root of the problem is that you're even allowed to push directly to the master branch on a shared repository. You should be pulling from the shared repo, but pushing to your own fork and making pull requests. Your pull requests should be reviewed by someone else before being merged by somebody other than you. Yes, you could avoid pushing to master by creating your own branch early on, but the real problem is that you (and presumably others) can push directly to master at all.

How bad is this practice, in terms of risk, and how could it be improved upon?

It's pretty bad, mostly because (again) you shouldn't be allowed to do it at all. How do you know that your "fix" is the right one? What do you do if someone else merges their changes (which may or may not have been reviewed) on top of yours before you realize that you screwed up?

The fix is to disallow pushes directly to master.

Update: Given your comment that you're not talking about the master branch in your shared repo but rather the master branch in your own fork, then your solution seems fine. The master branch in your own repo doesn't really mean much, at least for most of the work flows that I've seen. Usually you work in your own branch, then push that branch up to your fork and make a pull request back to the shared repo, at which point there should be a code review and maybe other checks before your code is merged. So your process to fix your own master branch is fine -- save your work off to a new branch, then reset or rebase your master branch to get it back in sync with the remote master.

Caleb
  • 124,013
  • 19
  • 183
  • 272
  • Apologies, caleb. I didn't explain that we use gerrit and other trickery/hookery to prevent direct modification of the remote master branch. I've tried to edit the question to clarify this. – Tim Randall Oct 23 '18 at 15:18
  • @TimRandall In that case there's really not a big problem at all... the master branch in your own fork shouldn't count for much. Pull from the remote master so that you're up to date, then create a branch for your work, then push your branch up to your fork and make a pull request from that branch back to remote master. If you use a different work flow, you might want to explain what it is in your question. – Caleb Oct 23 '18 at 15:23