Table of Contents
Git Procedures for Google Code In
Once Upon a Time, we had students send pull requests directly to the main repository. This led to voluminous and frequent interruptions of the folks who approve submissions to the main repository. We would like to avoid this. Our intent is to have mentors do complete code reviews of students' work, and assure that submissions work properly, pass tests, are correctly formatted, have no conflicts, and are mergeable. We would also like to batch the submissions to the main repository -- combine students' work over several days into one already-reviewed and tested bundle, and submit that as one PR.
After some head-scratching, discussion, and trial runs with help of GCI students, here is what seems to be a workable option. It is designed to avoid issues we encountered with a less formal procedure, in which mentors and students were freely updating from the main repository, without coordination.
We will be using a GitHub repository just for GCI, rather than submitting code directly to the main repository.
The GCI repository is:
Students will still fork the main repository, looking forward to when they will (we hope) be contributing beyond GCI.
However, during GCI, they will submit PRs to the GCI repository. In order to avoid chaos, we will temporarily freeze the revision level on which students will work -- we will provide a staging branch that students will pull from when they start work on a new task, and to which they will submit pull requests.
We will also have a base branch that shows the revision that was current in the main repository's master branch at the time we started a new cycle of aggregating student work on the staging branch. This is present purely for convenience in identifying the common base revision between the staging branch and the main repository's master branch. Students will not need to do anything with the base branch.
(By tradition in some popular git workflows, the master branch remains a mirror of the parent repository. To avoid disturbing that tradition, we are using different names for our branches.)
Since all student work during an aggregation cycle goes to the same staging branch, other students' work will likely be merged into the staging branch while a student's PR is being reviewed, so by the time the PR is ready, there may be a conflict with the other students' work. The mentor will help the student with the appropriate git procedure for rebasing and resolving their conflict. This is much like the normal workflow for submitting to the main repository, so students will get practice with the normal git workflow.
Every few days, we will switch to fresh base and staging branches: rename the current base and staging to (say) base_done and staging_done, and create new base and staging branches from the main repository's master. This is the only point at which coordination with students is needed. Since we don't want to delay preparing the completed code for a PR to the main repository, if there are any PRs in progress, we can close those before we rename the base and staging branches, and ask students to rebase from the new staging and re-open their PR there. As the students may have merge conflicts at this point, we can't automate this.
At this point, students' completed PRs are on the staging_done branch. A mentor will prepare the staging_done branch for a PR to the main repository: If the staging branch is not mergeable, the mentor can pull the staging branch to their local repository, merge from the main repository, fix up conflicts, and push to the staging branch again. (Note rebase is probably not appropriate here, as that would require modifying students' commits. It is better to allow students to have their work visible unmodified.) In any case, the combined code needs to be tested -- both running the test suite and verifying that the changes still work. (To facilitate this, we can have students write what should be done to exercise their change. We can rotate this mentor task, or free up one mentor from other work, and allow them to be the git-meister, as this step may take some time.)
Everyone involved -- students with submitted code and the mentors who reviewed it -- can help get that PR accepted into the main repository.
A separate repo has been created:
Branch "base" is a temporarily frozen revision level that is used only to record the revision after which the staging branch diverged from the main repo.
Branch "staging" is the branch students should pull, to get a local branch for new tasks, and is the destination for student pull requests.
Both will be periodically set to the same revision as master in the main repository.
Normal code review
This is essentially identical to a normal GitHub code review. This page is not about conducting the code review itself, only about changed git procedures due to using a separate GCI repository. You've all likely had the experience of being code-reviewed on GitHub, but we can add another page with code review tips if that would be useful.
- Once the review is done, and you're satisfied with the student's work, ask them to squash their commits and rebase from the staging branch.
- If their description of what their change does is not clear, ask them to post a comment describing the change briefly.
- If they don't have it already, ask them to add a comment telling how to use their change in the Eden UI, so it can be tested by hand.
- The big difference here is that you will be the one poking that friendly green "merge" button. If the button isn't green, and doesn't say the pull request can be automatically merged, then the student will need to rebase from the staging branch again, and fix the conflicts. (Note the student should rebase from staging even if the PR can be automatically merged, as that can be true even if the staging branch has changes past the revision at which the student pulled their branch. Doing one last rebase will put the commits in time order, with the student's new commit after the others on the staging branch, and avoid an internal non-linear "merge commit".)
- You may need to assist students with interactive rebase to squash their commits, with rebasing from the staging branch, and with fixing conflicts during the rebase, if it's the first time they've done it.
Also see the student procedures, so you know what they know.
This is where it gets interesting... Here are the steps to prepare student work for a PR to the main repository, and to switch to fresh base and staging branches for additional student work.
- Make sure there are no branches called base_done and staging_done in the edengci repository. That would mean the previous aggregation cycle is incomplete, or the branches were not cleaned up. Talk to the other mentors to find out what happened. When that's resolved...
- If there are any open student PRs:
- Post a note to the student GCI announcements thread in the sahana-eden mailing list asking students to pause in sending PRs until you say it's ok. (We'll add a link here to the announcements thread when it's ready.)
- Open the web page for each PR, as you'll need to post comments there after they are closed.
- Post a comment in each open PR, telling the student that you're starting an aggregation cycle, and are going to temporarily close their PR. They should watch for another comment or post in the announcements thread, and then open their PR again on the (new) staging branch, and rebase it from staging.
- Close the open PRs.
- Make one last check for open PRs in case any snuck in...
- Rename the base and staging branches on GitHub, as follows:
- If you have local branches called base or staging, rename or delete them, e.g.:
git branch -m staging staging_old git branch -m base base_old
- Pull copies of base and staging from the edengci repository.
git pull edengci staging git pull ededgci base
- Rename them.
git branch -m staging staging_done git branch -m base base_done
- Push to their new names.
git push edengci staging_done git push edengci base_done
- If you have local branches called base or staging, rename or delete them, e.g.:
- Make fresh base and staging branches. (The commands show assume that "upstream" is the name of the remote
you use for the main repository. If not, use whatever your remote name is.)
git pull upstream master:base git push edengci base git push edengci base:staging
- At this point, the new staging branch is ready for business. Post on the GCI announcement thread that students can re-open any PRs that had to be closed to rename staging, and can add new PRs.
- Post a comment in each closed PR telling the student they should now rebase their work from the (new) staging branch, fix any conflicts, and do another PR.
- Next, prepare for the PR to the main repository. Check out staging_done.
git checkout staging_done
- Run the test suite on your local copy of staging_done. If all has gone well with individual code reviews, there should be no problem here. But if there is, work with the student whose commit has the problem to resolve it. The student may need to do a PR to staging_done. Or you may want to fix the issue if it is minor and not due to a problem with the student's work. Don't bother with attempting to squash commits here, as we want to retain the student names as authors of the their commits.
- Run Eden from the staging_done branch. Go through the students' PR pages, and look for their comment about how to observe their change. Do that, and check that it still works. Again, if there's a problem, work with the student to fix it.
- If there were any fixups needed, push them to the staging_done branch.
- Do a PR to the main repository. Add a comment with the PR that tells what is in it -- should be able to scrape these from the students' PRs. As a courtesy, also scrape the student's instructions for testing -- Mark this so it's easily distinguished from the descriptive comment.
We will be using a GitHub repository just for GCI, rather than submitting code directly to the main repository. This will help us conserve the time of the (very busy) folks who have to approve submissions to the main repository.
You'll use the GCI repository to use for pull requests.
But you should still fork the main repository to create your own repository, and then clone your own repository.
On your local (cloned) repository, set up a "remote" that points to the GCI repository. Open the repository page:
On the right, there will be a box that says "SSH clone URL" or "HTTPS clone URL" -- that will be the URL to use for your new remote. If you are using git from the command line, cd to your local eden repository, and do this if you are using a password to connect to GitHub (this is the "HTTPS clone URL"):
git remote add edengci https://github.com/edengci/eden.git
Or do this if you are using SSH keys to connect to GitHub (this is the "SSH clone URL"):
git remote add edengci email@example.com:edengci/eden.git
We are going to collect up student work for several days, and then test it and submit it all together to the main repository. Because of that, we won't update from the main repository during that time. This will fix the problem students saw with having extra commits from the main repository that were not in the GCI repository appear in their pull requests.
We are going to use a branch called "staging" to hold students' work, so when you are ready to start working on code, you can pull a copy of this branch to your local repository. This is where you will send pull requests. To get a copy of this branch:
git pull edengci staging:your-branch-name
You can call your local branch whatever you want. One suggestion is to pull a fresh copy of the staging branch when you are about to start a new task. (You can keep any old branch you were working on for previous tasks, or delete it after your pull request for the previous task is accepted and merged into the staging branch. But it is better not to reuse that branch to avoid unexpected merge issues with new work. If you want to use the same branch name, then you can rename your old branch.)
Note, your commits will still have your name on them as author when they get into the main repository!
The steps to submit your work are similar to those in the git wiki page linked above -- differences are noted here.
- "Squash" your commits into one.
- This is different: Don't rebase from the main repository -- instead, rebase *from the staging branch* -- the same branch you will send the pull request to.
git pull --rebase edengci staging
- Push your commit to your Eden repository on GitHub.
- Open the GitHub page for your Eden repository.
- Click the "Compare and Create Pull Request" button shown on that commit.
- This is different: The pull request page has a line that shows which repository and branch you are sending the pull request too.
You will need to change both, to the GCI repository and the "staging" branch.
- There will be a down-arrow next to the repository -- that will give you a list of repositories. Select the GCI repository.
- Then use the down-arrow on the branch -- select "staging".
- Create the pull request.
- Continue with the code review as usual.
Every few days, we will send your completed work to the main repository. In order to do this, we will need to freeze work on the staging branch, so we can use that for a pull request to the main repository. So if you happen to have a pull request open to the staging branch when this happens, we will temporarily close your pull request. Then after we have made a new staging branch with the latest updates from the main repository, we will ask you to rebase your work from the new staging branch, and open a new pull request there. The command to rebase is identical to the one above (that you would do if there is a conflict during your code review).