search

otr4j / otr4j-issues

1 stars 0 forks source link

Confusion with tag-team git workflow (?) #7

Closed cobratbq closed 4 years ago

cobratbq commented 9 years ago

I want to check something with you. I just reviewed a trivial PR from languitar. (Partly to see how the tag-team git workflow works.) Now I have checked and pushed his commit to otr4j/otr4j. But how can I verify, apart from checking the github PR-info, that languitar didn't push it himself?

https://github.com/otr4j/otr4j/pull/17 shows that I merged it, however one cannot see that from the git commit log. I used the following steps:

  1. Verify by checking out: git checkout languitar/fix-pom-url (and doing actual verification)
  2. Push with: git push upstream languitar/fix-pom-url:master

Since I do not actually merge the PR, it doesn't reflect this in the commit log. Only because github tracks sources, can it show that this happened. (I checked gitk for commit information.)

eighthave commented 9 years ago

You can only verify that by checking before you merged whether @languitar's commit is on upstream/master. If there is a merge commit, that will have your user info in the commit, but that's not verified info. languitar (not to pick on him, just to continue the example) could have created that merge commit himself and added your user info to it.

That's why I think it is important to use git remotes, that makes it easy to see the status of commits in given remotes, including otr4j/otr4j. That's generally easy to do when using git remotes and git fetch --all. When I want to see the status of everyone's work, I make sure that everyone who's work I want to follow is added as a remote, then update all the remotes:

git remote add languitar https://github.com/languitar/otr4j
git fetch -all
gitk &

In gitk, I can easily see the status of everyone's branches using a handy "everything" view that I setup and saved on the "View" menu: gitk-everthing-view otr4j-git-remotes

cobratbq commented 9 years ago

Sure. I was asking mostly from an "auditing" pov and indeed languitar could simply fake my credentials. (At least until we start signing commits, that is.) I'll use non-ff merges such that at least there's always 2 names in a commit. I am clear on how to use the multiple remotes and viewing heads. I use the same method myself. I was asking just to make sure that pushing is a desirable action given the lack of traceability of the actual committer.

eighthave commented 9 years ago

About the auditing idea, that happens when you bring in any commit into your own local git repo. On a small project like this, I review all of the commits that I'm pulling into master on my own local git, so that's where I'm doing the auditing. Then when I have stuff that is ready to be pushed upstream, I do it from my local master.

I'm not really a fan of unneeded merge commits since they can make the git history quite messy. But if others like them, I can live with them. If we want to document who merged the pull request, we can also mark that info in the pull request itself.

I suppose we could do signed merge commits to close pull requests, then the merge commits would have a point. But I think that workflow might be too elaborate for many contributors.

cobratbq commented 9 years ago

Okay, so I just merged some PRs. That makes for an ugly history graph. (A bit worse than I expected.) I'll just go with branch pushing for simple cases. Thanks for the information. I wanted to be sure that I wasn't misbehaving in the suggested git workflow.