Paul Hammant's Blog:
Branches should be for change of policy on the same source
I saw Google’s git-appraise on Proggit yesterday. Hackernews had more comments for the same.
Super-cool - collaborative code review stored in source-control. I’ve been wishing for this for a long time. Here’s how it works:
$ git clone git@github.com:google/git-appraise.git
Cloning into 'git-appraise'...
remote: Counting objects: 543, done.
remote: Compressing objects: 100% (4/4), done.
remote: Total 543 (delta 0), reused 0 (delta 0), pack-reused 539
Receiving objects: 100% (543/543), 169.78 KiB | 0 bytes/s, done.
Resolving deltas: 100% (265/265), done.
Checking connectivity... done.
$ cd git-appraise/
$ git appraise list
Loaded 0 open reviews:
$ git fetch origin refs/notes/*:refs/notes/*
remote: Counting objects: 833, done.
remote: Compressing objects: 100% (42/42), done.
remote: Total 833 (delta 21), reused 0 (delta 0), pack-reused 791
Receiving objects: 100% (833/833), 117.92 KiB | 0 bytes/s, done.
Resolving deltas: 100% (398/398), done.
From github.com:google/git-appraise
* [new ref] refs/notes/devtools/analyses -> refs/notes/devtools/analyses
* [new ref] refs/notes/devtools/ci -> refs/notes/devtools/ci
* [new ref] refs/notes/devtools/discuss -> refs/notes/devtools/discuss
* [new ref] refs/notes/devtools/reviews -> refs/notes/devtools/reviews
$ git appraise list
Loaded 1 open reviews:
[pending] 2c9bff89f0f8
Improve the error messages returned when a git command fails.
Previously, we were simply cascading the error returned by the instance
of exec.Command. However, that winds up just being something of the form
"exit status 128", with all of the real error message going to the
Stderr field.
As such, this commit changes the behavior to save the data written to
stderr, and use it to construct a new error to return.
Git-appraise’s implementation though isn’t using a normal branch with text resources for the review which would be my preference. It is using refs/notes/devtools/reviews as a store. The branch list only shows master and a feature branch:
Indeed, the Github interface should allow you to navigate those text resources in the branch. I’ve written about this before. Ideally I would also be able to mount a third-party plugin to Github’s web UI and have a meaningful navigation of the review(s).
Branches or Repos?
The classic best practice is that branches should be for change of policy on the same source. Policy can be any/all of:
- groups can/can’t view branch versus other branch
- groups can/can’t check in to branch versus other branch
- branch contains stuff for a specific release (and no further)
- branch contains a work in progress on some functional or non-functional change, with variable level of granularity
Broadly speaking, branches should always be mergeable between each other, with the policy being the guide as to whether you should/shouldn’t (can/can’t if your SCM has permissions to some degree).
Github pages (branches)
The suggestion for Github Pages use, is to have it on a gh-pages branch, adjacent to your master branch (that contains your code). Sure, for a pure blog or site you might only have the Github Pages content and no programming source at all. Indeed, in that configuration you don’t need the gh-pages branch at all. I find the gh-pages branch a little bit odd really, however wonderful the technologies are. Merging between the gh-paged branch and the master being illogical is my persistent thought.
As the title says, branches should be for change of policy on the same source.
Github’s wiki implemention (repos)
Github implemented Wiki differently: as a first class repo, that follows a naming convention association with the repo it is associated with. D3’s repo can be cloned from git@github.com:mbostock/d3.git
and its wiki from https://github.com/mbostock/d3.wiki.git
. This is great. Less than great is the the fact that the Github web interface doesn’t allow you to navigate the revisions of pages, or see diffs etc. That would be so easy for them to implement. This is how Github Pages should have been implemented, in my opinion.
In summary, the wiki implementation is done right, given it is not a branch.
Git appraise (neither)
It’s being stored in refs/notes/devtools/reviews as mentioned. My thoughts though:
- Should be a first class SCMed set of text files, in my opinion, allowing merge amongst other things
- Would be best not represented as a branch with a different policy to the thing it is reviewing
- Should have be navigable in the same web-UI as the regular code
Comments formerly in Disqus, but exported and mounted statically ...
Tue, 15 Dec 2015 | Thomas Broyer |
Re. “Should be a first class SCMed set of text files” git-appraise reviews *are* first-class Git objects: https://git-scm.com/docs/gi... I fail to see when you'd want to do merges on review comments though… | |
Tue, 15 Dec 2015 | paul_hammant |
Take a look at https://paulhammant.com/2013... .. about 1/3 down - there's JSON files pertinent to a review in progress that could receive increments from different people. | |
Wed, 16 Dec 2015 | Thomas Broyer |
This is about contributing to the same file, not "merging" (in the sense of merging branches; sure there are merges happening when pulling and pushing, but that's a bit of a special case IMO). Anyway, git-notes supports all of that so… (and git-appraise is only a wrapper around Git and git-notes, wrt notes refs and JSON content; the diff itself is a Git commit, no need to store it as JSON as in your earlier proposal) BTW, Gerrit 3 will also move from storing review comments in a database to storing them into the Git repository (probably as git notes too). It already as a reviewnotes plugin that stores infos about reviews in git-notes, but that's only metadata, not the reviews themselves: https://gerrit.googlesource... | |
Wed, 16 Dec 2015 | Max Kirillov |
Well, git notes is a "standard" way to attach mutable information to commits. And they support somehow merges. But in general yes they don't have much attention and often not well supported by thirdparty tools. Hope this software is one more step to fix it. |